Merge pull request #1454 from github/dbartol/join-order

Report suspicious join orders

extensions/ql-vscode/package-lock.json

@@ -21,6 +21,7 @@
         "d3-graphviz": "^2.6.1",
         "fs-extra": "^10.0.1",
         "glob-promise": "^4.2.2",
+        "immutable": "^4.0.0",
         "js-yaml": "^4.1.0",
         "minimist": "~1.2.6",
         "nanoid": "^3.2.0",
@@ -7403,6 +7404,11 @@
         "node": ">= 4"
       }
     },
+    "node_modules/immutable": {
+      "version": "4.0.0",
+      "resolved": "https://registry.npmjs.org/immutable/-/immutable-4.0.0.tgz",
+      "integrity": "sha512-zIE9hX70qew5qTUjSS7wi1iwj/l7+m54KWU247nhM3v806UdGj1yDndXj+IOYxxtW9zyLI+xqFNZjTuDaLUqFw=="
+    },
     "node_modules/import-fresh": {
       "version": "3.2.1",
       "resolved": "https://registry.npmjs.org/import-fresh/-/import-fresh-3.2.1.tgz",
@@ -20516,6 +20522,11 @@
       "integrity": "sha512-cyFDKrqc/YdcWFniJhzI42+AzS+gNwmUzOSFcRCQYwySuBBBy/KjuxWLZ/FHEH6Moq1NizMOBWyTcv8O4OZIMg==",
       "dev": true
     },
+    "immutable": {
+      "version": "4.0.0",
+      "resolved": "https://registry.npmjs.org/immutable/-/immutable-4.0.0.tgz",
+      "integrity": "sha512-zIE9hX70qew5qTUjSS7wi1iwj/l7+m54KWU247nhM3v806UdGj1yDndXj+IOYxxtW9zyLI+xqFNZjTuDaLUqFw=="
+    },
     "import-fresh": {
       "version": "3.2.1",
       "resolved": "https://registry.npmjs.org/import-fresh/-/import-fresh-3.2.1.tgz",

extensions/ql-vscode/package.json

@@ -1211,6 +1211,7 @@
     "d3-graphviz": "^2.6.1",
     "fs-extra": "^10.0.1",
     "glob-promise": "^4.2.2",
+    "immutable": "^4.0.0",
     "js-yaml": "^4.1.0",
     "minimist": "~1.2.6",
     "nanoid": "^3.2.0",

extensions/ql-vscode/src/cli.ts

@@ -695,7 +695,7 @@ export class CodeQLCliServer implements Disposable {
   * @param inputPath The path of an evaluation event log.
   * @param outputPath The path to write a JSON summary of it to.
   */
-   async generateJsonLogSummary(
+  async generateJsonLogSummary(
     inputPath: string,
     outputPath: string,
   ): Promise<string> {

extensions/ql-vscode/src/extension.ts

@@ -100,6 +100,8 @@ import { exportRemoteQueryResults } from './remote-queries/export-results';
 import { RemoteQuery } from './remote-queries/remote-query';
 import { EvalLogViewer } from './eval-log-viewer';
 import { SummaryLanguageSupport } from './log-insights/summary-language-support';
+import { JoinOrderScannerProvider } from './log-insights/join-order';
+import { LogScannerService } from './log-insights/log-scanner-service';
 
 /**
  * extension.ts
@@ -483,6 +485,11 @@ async function activateWithInstalledDistribution(
 
   ctx.subscriptions.push(qhm);
 
+  void logger.log('Initializing evaluation log scanners.');
+  const logScannerService = new LogScannerService(qhm);
+  ctx.subscriptions.push(logScannerService);
+  ctx.subscriptions.push(logScannerService.scanners.registerLogScannerProvider(new JoinOrderScannerProvider()));
+
   void logger.log('Reading query history');
   await qhm.readQueryHistory();
 
@@ -556,7 +563,7 @@ async function activateWithInstalledDistribution(
           undefined,
           item,
         );
-        item.completeThisQuery(completedQueryInfo);
+        qhm.completeQuery(item, completedQueryInfo);
         await showResultsForCompletedQuery(item as CompletedLocalQueryInfo, WebviewReveal.NotForced);
         // Note we must update the query history view after showing results as the
         // display and sorting might depend on the number of results

extensions/ql-vscode/src/log-insights/join-order.ts

@@ -0,0 +1,460 @@
+import * as I from 'immutable';
+import { EvaluationLogProblemReporter, EvaluationLogScanner, EvaluationLogScannerProvider } from './log-scanner';
+import { InLayer, ComputeRecursive, SummaryEvent, PipelineRun, ComputeSimple } from './log-summary';
+
+const DEFAULT_WARNING_THRESHOLD = 50;
+
+/**
+ * Like `max`, but returns 0 if no meaningful maximum can be computed.
+ */
+function safeMax(it: Iterable<number>) {
+  const m = Math.max(...it);
+  return Number.isFinite(m) ? m : 0;
+}
+
+/**
+ * Compute a key for the maps that that is sent to report generation.
+ * Should only be used on events that are known to define queryCausingWork.
+ */
+function makeKey(
+  queryCausingWork: string | undefined,
+  predicate: string,
+  suffix = ''
+): string {
+  if (queryCausingWork === undefined) {
+    throw new Error(
+      'queryCausingWork was not defined on an event we expected it to be defined for!'
+    );
+  }
+  return `${queryCausingWork}:${predicate}${suffix ? ' ' + suffix : ''}`;
+}
+
+const DEPENDENT_PREDICATES_REGEXP = (() => {
+  const regexps = [
+    // SCAN id
+    String.raw`SCAN\s+([0-9a-zA-Z:#_]+)\s`,
+    // JOIN id WITH id
+    String.raw`JOIN\s+([0-9a-zA-Z:#_]+)\s+WITH\s+([0-9a-zA-Z:#_]+)\s`,
+    // AGGREGATE id, id
+    String.raw`AGGREGATE\s+([0-9a-zA-Z:#_]+)\s*,\s+([0-9a-zA-Z:#_]+)`,
+    // id AND NOT id
+    String.raw`([0-9a-zA-Z:#_]+)\s+AND\s+NOT\s+([0-9a-zA-Z:#_]+)`,
+    // INVOKE HIGHER-ORDER RELATION rel ON <id, ..., id>
+    String.raw`INVOKE\s+HIGHER-ORDER\s+RELATION\s[^\s]+\sON\s+<([0-9a-zA-Z:#_<>]+)((?:,[0-9a-zA-Z:#_<>]+)*)>`,
+    // SELECT id
+    String.raw`SELECT\s+([0-9a-zA-Z:#_]+)`
+  ];
+  return new RegExp(
+    `${String.raw`\{[0-9]+\}\s+[0-9a-zA-Z]+\s=\s(?:` + regexps.join('|')})`
+  );
+})();
+
+function getDependentPredicates(operations: string[]): I.List<string> {
+  return I.List(operations).flatMap(operation => {
+    const matches = DEPENDENT_PREDICATES_REGEXP.exec(operation.trim());
+    if (matches !== null) {
+      return I.List(matches)
+        .rest() // Skip the first group as it's just the entire string
+        .filter(x => !!x && !x.match('r[0-9]+|PRIMITIVE')) // Only keep the references to predicates.
+        .flatMap(x => x.split(',')) // Group 2 in the INVOKE HIGHER_ORDER RELATION case is a comma-separated list of identifiers.
+        .filter(x => !!x); // Remove empty strings
+    } else {
+      return I.List();
+    }
+  });
+}
+
+function getMainHash(event: InLayer | ComputeRecursive): string {
+  switch (event.evaluationStrategy) {
+    case 'IN_LAYER':
+      return event.mainHash;
+    case 'COMPUTE_RECURSIVE':
+      return event.raHash;
+  }
+}
+
+/**
+ * Sum arrays a and b element-wise. The shorter array is padded with 0s if the arrays are not the same length.
+ */
+function pointwiseSum(a: Int32Array, b: Int32Array, problemReporter: EvaluationLogProblemReporter): Int32Array {
+  function reportIfInconsistent(ai: number, bi: number) {
+    if (ai === -1 && bi !== -1) {
+      problemReporter.log(
+        `Operation was not evaluated in the first pipeline, but it was evaluated in the accumulated pipeline (with tuple count ${bi}).`
+      );
+    }
+    if (ai !== -1 && bi === -1) {
+      problemReporter.log(
+        `Operation was evaluated in the first pipeline (with tuple count ${ai}), but it was not evaluated in the accumulated pipeline.`
+      );
+    }
+  }
+
+  const length = Math.max(a.length, b.length);
+  const result = new Int32Array(length);
+  for (let i = 0; i < length; i++) {
+    const ai = a[i] || 0;
+    const bi = b[i] || 0;
+    // -1 is used to represent the absence of a tuple count for a line in the pretty-printed RA (e.g. an empty line), so we ignore those.
+    if (i < a.length && i < b.length && (ai === -1 || bi === -1)) {
+      result[i] = -1;
+      reportIfInconsistent(ai, bi);
+    } else {
+      result[i] = ai + bi;
+    }
+  }
+  return result;
+}
+
+function pushValue<K, V>(m: Map<K, V[]>, k: K, v: V) {
+  if (!m.has(k)) {
+    m.set(k, []);
+  }
+  m.get(k)!.push(v);
+  return m;
+}
+
+function computeJoinOrderBadness(
+  maxTupleCount: number,
+  maxDependentPredicateSize: number,
+  resultSize: number
+): number {
+  return maxTupleCount / Math.max(maxDependentPredicateSize, resultSize);
+}
+
+/**
+ * A bucket contains the pointwise sum of the tuple counts, result sizes and dependent predicate sizes
+ * For each (predicate, order) in an SCC, we will compute a bucket.
+ */
+interface Bucket {
+  tupleCounts: Int32Array;
+  resultSize: number;
+  dependentPredicateSizes: I.Map<string, number>;
+}
+
+class JoinOrderScanner implements EvaluationLogScanner {
+  // Map a predicate hash to its result size
+  private readonly predicateSizes = new Map<string, number>();
+  private readonly layerEvents = new Map<string, (ComputeRecursive | InLayer)[]>();
+  // Map a key of the form 'query-with-demand : predicate name' to its badness input.
+  private readonly maxTupleCountMap = new Map<string, number[]>();
+  private readonly resultSizeMap = new Map<string, number[]>();
+  private readonly maxDependentPredicateSizeMap = new Map<string, number[]>();
+  private readonly joinOrderMetricMap = new Map<string, number>();
+
+  constructor(
+    private readonly problemReporter: EvaluationLogProblemReporter,
+    private readonly warningThreshold: number) {
+  }
+
+  public onEvent(event: SummaryEvent): void {
+    if (
+      event.completionType !== undefined &&
+      event.completionType !== 'SUCCESS'
+    ) {
+      return; // Skip any evaluation that wasn't successful
+    }
+
+    this.recordPredicateSizes(event);
+    this.computeBadnessMetric(event);
+  }
+
+  public onDone(): void {
+    void this;
+  }
+
+  private recordPredicateSizes(event: SummaryEvent): void {
+    switch (event.evaluationStrategy) {
+      case 'EXTENSIONAL':
+      case 'COMPUTED_EXTENSIONAL':
+      case 'COMPUTE_SIMPLE':
+      case 'CACHACA':
+      case 'CACHE_HIT': {
+        this.predicateSizes.set(event.raHash, event.resultSize);
+        break;
+      }
+      case 'SENTINEL_EMPTY': {
+        this.predicateSizes.set(event.raHash, 0);
+        break;
+      }
+      case 'COMPUTE_RECURSIVE':
+      case 'IN_LAYER': {
+        this.predicateSizes.set(event.raHash, event.resultSize);
+        // layerEvents are indexed by the mainHash.
+        const hash = getMainHash(event);
+        if (!this.layerEvents.has(hash)) {
+          this.layerEvents.set(hash, []);
+        }
+        this.layerEvents.get(hash)!.push(event);
+        break;
+      }
+    }
+  }
+
+  private reportProblemIfNecessary(event: SummaryEvent, iteration: number, metric: number): void {
+    if (metric >= this.warningThreshold) {
+      this.problemReporter.reportProblem(event.predicateName, event.raHash, iteration,
+        `Relation '${event.predicateName}' has an inefficient join order. Its join order metric is ${metric.toFixed(2)}, which is larger than the threshold of ${this.warningThreshold.toFixed(2)}.`);
+    }
+  }
+
+  private computeBadnessMetric(event: SummaryEvent): void {
+    if (
+      event.completionType !== undefined &&
+      event.completionType !== 'SUCCESS'
+    ) {
+      return; // Skip any evaluation that wasn't successful
+    }
+    switch (event.evaluationStrategy) {
+      case 'COMPUTE_SIMPLE': {
+        if (!event.pipelineRuns) {
+          // skip if the optional pipelineRuns field is not present.
+          break;
+        }
+        // Compute the badness metric for a non-recursive predicate. The metric in this case is defined as:
+        // badness = (max tuple count in the pipeline) / (largest predicate this pipeline depends on)
+        const key = makeKey(event.queryCausingWork, event.predicateName);
+        const resultSize = event.resultSize;
+
+        // There is only one entry in `pipelineRuns` if it's a non-recursive predicate.
+        const { maxTupleCount, maxDependentPredicateSize } =
+          this.badnessInputsForNonRecursiveDelta(event.pipelineRuns[0], event);
+
+        if (maxDependentPredicateSize > 0) {
+          pushValue(this.maxTupleCountMap, key, maxTupleCount);
+          pushValue(this.resultSizeMap, key, resultSize);
+          pushValue(
+            this.maxDependentPredicateSizeMap,
+            key,
+            maxDependentPredicateSize
+          );
+          const metric = computeJoinOrderBadness(maxTupleCount, maxDependentPredicateSize, resultSize!);
+          this.joinOrderMetricMap.set(key, metric);
+          this.reportProblemIfNecessary(event, 0, metric);
+        }
+        break;
+      }
+
+      case 'COMPUTE_RECURSIVE': {
+        // Compute the badness metric for a recursive predicate for each ordering.
+        const sccMetricInput = this.badnessInputsForRecursiveDelta(event);
+        // Loop through each predicate in the SCC
+        sccMetricInput.forEach((buckets, predicate) => {
+          // Loop through each ordering of the predicate
+          buckets.forEach((bucket, raReference) => {
+            // Format the key as demanding-query:name (ordering)
+            const key = makeKey(
+              event.queryCausingWork,
+              predicate,
+              `(${raReference})`
+            );
+            const maxTupleCount = Math.max(...bucket.tupleCounts);
+            const resultSize = bucket.resultSize;
+            const maxDependentPredicateSize = Math.max(
+              ...bucket.dependentPredicateSizes.values()
+            );
+
+            if (maxDependentPredicateSize > 0) {
+              pushValue(this.maxTupleCountMap, key, maxTupleCount);
+              pushValue(this.resultSizeMap, key, resultSize);
+              pushValue(
+                this.maxDependentPredicateSizeMap,
+                key,
+                maxDependentPredicateSize
+              );
+              const metric = computeJoinOrderBadness(maxTupleCount, maxDependentPredicateSize, resultSize);
+              const oldMetric = this.joinOrderMetricMap.get(key);
+              if ((oldMetric === undefined) || (metric > oldMetric)) {
+                this.joinOrderMetricMap.set(key, metric);
+              }
+            }
+          });
+        });
+        break;
+      }
+    }
+  }
+
+  /**
+   * Iterate through an SCC with main node `event`.
+   */
+  private iterateSCC(
+    event: ComputeRecursive,
+    func: (
+      inLayerEvent: ComputeRecursive | InLayer,
+      run: PipelineRun,
+      iteration: number
+    ) => void
+  ): void {
+    const sccEvents = this.layerEvents.get(event.raHash)!;
+    const nextPipeline: number[] = new Array(sccEvents.length).fill(0);
+
+    const maxIteration = Math.max(
+      ...sccEvents.map(e => e.predicateIterationMillis.length)
+    );
+
+    for (let iteration = 0; iteration < maxIteration; ++iteration) {
+      // Loop through each predicate in this iteration
+      for (let predicate = 0; predicate < sccEvents.length; ++predicate) {
+        const inLayerEvent = sccEvents[predicate];
+        const iterationTime =
+          inLayerEvent.predicateIterationMillis.length <= iteration
+            ? -1
+            : inLayerEvent.predicateIterationMillis[iteration];
+        if (iterationTime != -1) {
+          const run: PipelineRun =
+            inLayerEvent.pipelineRuns[nextPipeline[predicate]++];
+          func(inLayerEvent, run, iteration);
+        }
+      }
+    }
+  }
+
+  /**
+   * Compute the maximum tuple count and maximum dependent predicate size for a non-recursive pipeline
+   */
+  private badnessInputsForNonRecursiveDelta(
+    pipelineRun: PipelineRun,
+    event: ComputeSimple
+  ): { maxTupleCount: number; maxDependentPredicateSize: number } {
+    const dependentPredicateSizes = Object.values(event.dependencies).map(hash =>
+      this.predicateSizes.get(hash) ?? 0  // Should always be present, but zero is a safe default.
+    );
+    const maxDependentPredicateSize = safeMax(dependentPredicateSizes);
+    return {
+      maxTupleCount: safeMax(pipelineRun.counts),
+      maxDependentPredicateSize: maxDependentPredicateSize
+    };
+  }
+
+  private prevDeltaSizes(event: ComputeRecursive, predicate: string, i: number) {
+    // If an iteration isn't present in the map it means it was skipped because the optimizer
+    // inferred that it was empty. So its size is 0.
+    return this.curDeltaSizes(event, predicate, i - 1);
+  }
+
+  private curDeltaSizes(event: ComputeRecursive, predicate: string, i: number) {
+    // If an iteration isn't present in the map it means it was skipped because the optimizer
+    // inferred that it was empty. So its size is 0.
+    return (
+      this.layerEvents.get(event.raHash)?.find(x => x.predicateName === predicate)?.deltaSizes[i] ?? 0
+    );
+  }
+
+  /**
+   * Compute the metric dependent predicate sizes and the result size for a predicate in an SCC.
+   */
+  private badnessInputsForLayer(
+    event: ComputeRecursive,
+    inLayerEvent: InLayer | ComputeRecursive,
+    raReference: string,
+    iteration: number
+  ) {
+    const dependentPredicates = getDependentPredicates(
+      inLayerEvent.ra[raReference]
+    );
+    let dependentPredicateSizes: I.Map<string, number>;
+    // We treat the base case as a non-recursive pipeline. In that case, the dependent predicates are
+    // the dependencies of the base case and the cur_deltas.
+    if (raReference === 'base') {
+      dependentPredicateSizes = I.Map(
+        dependentPredicates.map((pred): [string, number] => {
+          // A base case cannot contain a `prev_delta`, but it can contain a `cur_delta`.
+          let size = 0;
+          if (pred.endsWith('#cur_delta')) {
+            size = this.curDeltaSizes(
+              event,
+              pred.slice(0, -'#cur_delta'.length),
+              iteration
+            );
+          } else {
+            const hash = event.dependencies[pred];
+            size = this.predicateSizes.get(hash)!;
+          }
+          return [pred, size];
+        })
+      );
+    } else {
+      // It's a non-base case in a recursive pipeline. In that case, the dependent predicates are
+      // only the prev_deltas.
+      dependentPredicateSizes = I.Map(
+        dependentPredicates
+          .flatMap(pred => {
+            // If it's actually a prev_delta
+            if (pred.endsWith('#prev_delta')) {
+              // Return the predicate without the #prev_delta suffix.
+              return [pred.slice(0, -'#prev_delta'.length)];
+            } else {
+              // Not a recursive delta. Skip it.
+              return [];
+            }
+          })
+          .map((prev): [string, number] => {
+            const size = this.prevDeltaSizes(event, prev, iteration);
+            return [prev, size];
+          })
+      );
+    }
+
+    const deltaSize = inLayerEvent.deltaSizes[iteration];
+    return { dependentPredicateSizes, deltaSize };
+  }
+
+  /**
+   * Compute the metric input for all the events in a SCC that starts with main node `event`
+   */
+  private badnessInputsForRecursiveDelta(event: ComputeRecursive): Map<string, Map<string, Bucket>> {
+    // nameToOrderToBucket : predicate name -> ordering (i.e., standard, order_500000, etc.) -> bucket
+    const nameToOrderToBucket = new Map<string, Map<string, Bucket>>();
+
+    // Iterate through the SCC and compute the metric inputs
+    this.iterateSCC(event, (inLayerEvent, run, iteration) => {
+      const raReference = run.raReference;
+      const predicateName = inLayerEvent.predicateName;
+      if (!nameToOrderToBucket.has(predicateName)) {
+        nameToOrderToBucket.set(predicateName, new Map());
+      }
+      const orderTobucket = nameToOrderToBucket.get(predicateName)!;
+      if (!orderTobucket.has(raReference)) {
+        orderTobucket.set(raReference, {
+          tupleCounts: new Int32Array(0),
+          resultSize: 0,
+          dependentPredicateSizes: I.Map()
+        });
+      }
+
+      const { dependentPredicateSizes, deltaSize } = this.badnessInputsForLayer(
+        event,
+        inLayerEvent,
+        raReference,
+        iteration
+      );
+
+      const bucket = orderTobucket.get(raReference)!;
+      // Pointwise sum the tuple counts
+      const newTupleCounts = pointwiseSum(
+        bucket.tupleCounts,
+        new Int32Array(run.counts),
+        this.problemReporter
+      );
+      const resultSize = bucket.resultSize + deltaSize;
+      // Pointwise sum the deltas.
+      const newDependentPredicateSizes = bucket.dependentPredicateSizes.mergeWith(
+        (oldSize, newSize) => oldSize + newSize,
+        dependentPredicateSizes
+      );
+      orderTobucket.set(raReference, {
+        tupleCounts: newTupleCounts,
+        resultSize: resultSize,
+        dependentPredicateSizes: newDependentPredicateSizes
+      });
+    });
+    return nameToOrderToBucket;
+  }
+}
+
+export class JoinOrderScannerProvider implements EvaluationLogScannerProvider {
+  public createScanner(problemReporter: EvaluationLogProblemReporter): EvaluationLogScanner {
+    return new JoinOrderScanner(problemReporter, DEFAULT_WARNING_THRESHOLD);
+  }
+}

extensions/ql-vscode/src/log-insights/jsonl-reader.ts

@@ -0,0 +1,23 @@
+import * as fs from 'fs-extra';
+
+/**
+ * Read a file consisting of multiple JSON objects. Each object is separated from the previous one
+ * by a double newline sequence. This is basically a more human-readable form of JSONL.
+ *
+ * The current implementation reads the entire text of the document into memory, but in the future
+ * it will stream the document to improve the performance with large documents.
+ *
+ * @param path The path to the file.
+ * @param handler Callback to be invoked for each top-level JSON object in order.
+ */
+export async function readJsonlFile(path: string, handler: (value: any) => Promise<void>): Promise<void> {
+  const logSummary = await fs.readFile(path, 'utf-8');
+
+  // Remove newline delimiters because summary is in .jsonl format.
+  const jsonSummaryObjects: string[] = logSummary.split(/\r?\n\r?\n/g);
+
+  for (const obj of jsonSummaryObjects) {
+    const jsonObj = JSON.parse(obj);
+    await handler(jsonObj);
+  }
+}

extensions/ql-vscode/src/log-insights/log-scanner-service.ts

@@ -0,0 +1,109 @@
+import { Diagnostic, DiagnosticSeverity, languages, Range, Uri } from 'vscode';
+import { DisposableObject } from '../pure/disposable-object';
+import { QueryHistoryManager } from '../query-history';
+import { QueryHistoryInfo } from '../query-results';
+import { EvaluationLogProblemReporter, EvaluationLogScannerSet } from './log-scanner';
+import { PipelineInfo, SummarySymbols } from './summary-parser';
+import * as fs from 'fs-extra';
+import { logger } from '../logging';
+
+/**
+ * Compute the key used to find a predicate in the summary symbols.
+ * @param name The name of the predicate.
+ * @param raHash The RA hash of the predicate.
+ * @returns The key of the predicate, consisting of `name@shortHash`, where `shortHash` is the first
+ * eight characters of `raHash`.
+ */
+function predicateSymbolKey(name: string, raHash: string): string {
+  return `${name}@${raHash.substring(0, 8)}`;
+}
+
+/**
+ * Implementation of `EvaluationLogProblemReporter` that generates `Diagnostic` objects to display
+ * in the VS Code "Problems" view.
+ */
+class ProblemReporter implements EvaluationLogProblemReporter {
+  public readonly diagnostics: Diagnostic[] = [];
+
+  constructor(private readonly symbols: SummarySymbols | undefined) {
+  }
+
+  public reportProblem(predicateName: string, raHash: string, iteration: number, message: string): void {
+    const nameWithHash = predicateSymbolKey(predicateName, raHash);
+    const predicateSymbol = this.symbols?.predicates[nameWithHash];
+    let predicateInfo: PipelineInfo | undefined = undefined;
+    if (predicateSymbol !== undefined) {
+      predicateInfo = predicateSymbol.iterations[iteration];
+    }
+    if (predicateInfo !== undefined) {
+      const range = new Range(predicateInfo.raStartLine, 0, predicateInfo.raEndLine + 1, 0);
+      this.diagnostics.push(new Diagnostic(range, message, DiagnosticSeverity.Error));
+    }
+  }
+
+  public log(message: string): void {
+    void logger.log(message);
+  }
+}
+
+export class LogScannerService extends DisposableObject {
+  public readonly scanners = new EvaluationLogScannerSet();
+  private readonly diagnosticCollection = this.push(languages.createDiagnosticCollection('ql-eval-log'));
+  private currentItem: QueryHistoryInfo | undefined = undefined;
+
+  constructor(qhm: QueryHistoryManager) {
+    super();
+
+    this.push(qhm.onDidChangeCurrentQueryItem(async (item) => {
+      if (item !== this.currentItem) {
+        this.currentItem = item;
+        await this.scanEvalLog(item);
+      }
+    }));
+
+    this.push(qhm.onDidCompleteQuery(async (item) => {
+      if (item === this.currentItem) {
+        await this.scanEvalLog(item);
+      }
+    }));
+  }
+
+  /**
+   * Scan the evaluation log for a query, and report any diagnostics.
+   *
+   * @param query The query whose log is to be scanned.
+   */
+  public async scanEvalLog(
+    query: QueryHistoryInfo | undefined
+  ): Promise<void> {
+    this.diagnosticCollection.clear();
+
+    if ((query?.t !== 'local')
+      || (query.evalLogSummaryLocation === undefined)
+      || (query.jsonEvalLogSummaryLocation === undefined)) {
+      return;
+    }
+
+    const diagnostics = await this.scanLog(query.jsonEvalLogSummaryLocation, query.evalLogSummarySymbolsLocation);
+    const uri = Uri.file(query.evalLogSummaryLocation);
+    this.diagnosticCollection.set(uri, diagnostics);
+  }
+
+  /**
+   * Scan the evaluator summary log for problems, using the scanners for all registered providers.
+   * @param jsonSummaryLocation The file path of the JSON summary log.
+   * @param symbolsLocation The file path of the symbols file for the human-readable log summary.
+   * @returns An array of `Diagnostic`s representing the problems found by scanners.
+   */
+  private async scanLog(jsonSummaryLocation: string, symbolsLocation: string | undefined): Promise<Diagnostic[]> {
+    let symbols: SummarySymbols | undefined = undefined;
+    if (symbolsLocation !== undefined) {
+      symbols = JSON.parse(await fs.readFile(symbolsLocation, { encoding: 'utf-8' }));
+    }
+    const problemReporter = new ProblemReporter(symbols);
+
+    await this.scanners.scanLog(jsonSummaryLocation, problemReporter);
+
+    return problemReporter.diagnostics;
+  }
+}

extensions/ql-vscode/src/log-insights/log-scanner.ts

@@ -0,0 +1,103 @@
+import { SummaryEvent } from './log-summary';
+import { readJsonlFile } from './jsonl-reader';
+
+/**
+ * Callback interface used to report diagnostics from a log scanner.
+ */
+export interface EvaluationLogProblemReporter {
+  /**
+   * Report a potential problem detected in the evaluation log.
+   *
+   * @param predicateName The mangled name of the predicate with the problem.
+   * @param raHash The RA hash of the predicate with the problem.
+   * @param iteration The iteration number with the problem. For a non-recursive predicate, this
+   * must be zero.
+   * @param message The problem message.
+   */
+  reportProblem(predicateName: string, raHash: string, iteration: number, message: string): void;
+
+  /**
+   * Log a message about a problem in the implementation of the scanner. These will typically be
+   * displayed separate from any problems reported via `reportProblem()`.
+   */
+  log(message: string): void;
+}
+
+/**
+ * Interface implemented by a log scanner. Instances are created via
+ * `EvaluationLogScannerProvider.createScanner()`.
+ */
+export interface EvaluationLogScanner {
+  /**
+   * Called for each event in the log summary, in order. The implementation can report problems via
+   * the `EvaluationLogProblemReporter` interface that was supplied to `createScanner()`.
+   * @param event The log summary event.
+   */
+  onEvent(event: SummaryEvent): void;
+  /**
+   * Called after all events in the log summary have been processed. The implementation can report
+   * problems via the `EvaluationLogProblemReporter` interface that was supplied to
+   * `createScanner()`.
+   */
+  onDone(): void;
+}
+
+/**
+ * A factory for log scanners. When a log is to be scanned, all registered
+ * `EvaluationLogScannerProviders` will be asked to create a new instance of `EvaluationLogScanner`
+ * to do the scanning.
+ */
+export interface EvaluationLogScannerProvider {
+  /**
+   * Create a new instance of `EvaluationLogScanner` to scan a single summary log.
+   * @param problemReporter Callback interface for reporting any problems discovered.
+   */
+  createScanner(problemReporter: EvaluationLogProblemReporter): EvaluationLogScanner;
+}
+
+/**
+ * Same as VSCode's `Disposable`, but avoids a dependency on VS Code.
+ */
+export interface Disposable {
+  dispose(): void;
+}
+
+export class EvaluationLogScannerSet {
+  private readonly scannerProviders = new Map<number, EvaluationLogScannerProvider>();
+  private nextScannerProviderId = 0;
+
+  /**
+   * Register a provider that can create instances of `EvaluationLogScanner` to scan evaluation logs
+   * for problems.
+   * @param provider The provider.
+   * @returns A `Disposable` that, when disposed, will unregister the provider.
+   */
+  public registerLogScannerProvider(provider: EvaluationLogScannerProvider): Disposable {
+    const id = this.nextScannerProviderId;
+    this.nextScannerProviderId++;
+
+    this.scannerProviders.set(id, provider);
+    return {
+      dispose: () => {
+        this.scannerProviders.delete(id);
+      }
+    };
+  }
+
+  /**
+   * Scan the evaluator summary log for problems, using the scanners for all registered providers.
+   * @param jsonSummaryLocation The file path of the JSON summary log.
+   * @param problemReporter Callback interface for reporting any problems discovered.
+   */
+  public async scanLog(jsonSummaryLocation: string, problemReporter: EvaluationLogProblemReporter): Promise<void> {
+    const scanners = [...this.scannerProviders.values()].map(p => p.createScanner(problemReporter));
+
+    await readJsonlFile(jsonSummaryLocation, async obj => {
+      scanners.forEach(scanner => {
+        scanner.onEvent(obj);
+      });
+    });
+
+    scanners.forEach(scanner => scanner.onDone());
+  }
+}

extensions/ql-vscode/src/log-insights/log-summary.ts

@@ -0,0 +1,93 @@
+export interface PipelineRun {
+  raReference: string;
+  counts: number[];
+  duplicationPercentages: number[];
+}
+
+export interface Ra {
+  [key: string]: string[];
+}
+
+export type EvaluationStrategy =
+  'COMPUTE_SIMPLE' |
+  'COMPUTE_RECURSIVE' |
+  'IN_LAYER' |
+  'COMPUTED_EXTENSIONAL' |
+  'EXTENSIONAL' |
+  'SENTINEL_EMPTY' |
+  'CACHACA' |
+  'CACHE_HIT';
+
+interface SummaryEventBase {
+  evaluationStrategy: EvaluationStrategy;
+  predicateName: string;
+  raHash: string;
+  appearsAs: { [key: string]: { [key: string]: number[] } };
+  completionType?: string;
+}
+
+interface ResultEventBase extends SummaryEventBase {
+  resultSize: number;
+}
+
+export interface ComputeSimple extends ResultEventBase {
+  evaluationStrategy: 'COMPUTE_SIMPLE';
+  ra: Ra;
+  pipelineRuns?: [PipelineRun];
+  queryCausingWork?: string;
+  dependencies: { [key: string]: string };
+}
+
+export interface ComputeRecursive extends ResultEventBase {
+  evaluationStrategy: 'COMPUTE_RECURSIVE';
+  deltaSizes: number[];
+  ra: Ra;
+  pipelineRuns: PipelineRun[];
+  queryCausingWork?: string;
+  dependencies: { [key: string]: string };
+  predicateIterationMillis: number[];
+}
+
+export interface InLayer extends ResultEventBase {
+  evaluationStrategy: 'IN_LAYER';
+  deltaSizes: number[];
+  ra: Ra;
+  pipelineRuns: PipelineRun[];
+  queryCausingWork?: string;
+  mainHash: string;
+  predicateIterationMillis: number[];
+}
+
+export interface ComputedExtensional extends ResultEventBase {
+  evaluationStrategy: 'COMPUTED_EXTENSIONAL';
+  queryCausingWork?: string;
+}
+
+export interface NonComputedExtensional extends ResultEventBase {
+  evaluationStrategy: 'EXTENSIONAL';
+  queryCausingWork?: string;
+}
+
+export interface SentinelEmpty extends SummaryEventBase {
+  evaluationStrategy: 'SENTINEL_EMPTY';
+  sentinelRaHash: string;
+}
+
+export interface Cachaca extends ResultEventBase {
+  evaluationStrategy: 'CACHACA';
+}
+
+export interface CacheHit extends ResultEventBase {
+  evaluationStrategy: 'CACHE_HIT';
+}
+
+export type Extensional = ComputedExtensional | NonComputedExtensional;
+
+export type SummaryEvent =
+  | ComputeSimple
+  | ComputeRecursive
+  | InLayer
+  | Extensional
+  | SentinelEmpty
+  | Cachaca
+  | CacheHit;

extensions/ql-vscode/src/log-insights/summary-language-support.ts

@@ -35,11 +35,11 @@ export class SummaryLanguageSupport extends DisposableObject {
    * The last `TextDocument` (with language `ql-summary`) for which we tried to find a sourcemap, or
    * `undefined` if we have not seen such a document yet.
    */
-  private lastDocument : TextDocument | undefined = undefined;
+  private lastDocument: TextDocument | undefined = undefined;
   /**
    * The sourcemap for `lastDocument`, or `undefined` if there was no such sourcemap or document.
    */
-  private sourceMap : SourceMapConsumer | undefined = undefined;
+  private sourceMap: SourceMapConsumer | undefined = undefined;
 
   constructor() {
     super();

extensions/ql-vscode/src/log-insights/summary-parser.ts

@@ -0,0 +1,113 @@
+import * as fs from 'fs-extra';
+
+/**
+ * Location information for a single pipeline invocation in the RA.
+ */
+export interface PipelineInfo {
+  startLine: number;
+  raStartLine: number;
+  raEndLine: number;
+}
+
+/**
+ * Location information for a single predicate in the RA.
+ */
+export interface PredicateSymbol {
+  /**
+   * `PipelineInfo` for each iteration. A non-recursive predicate will have a single iteration `0`.
+   */
+  iterations: Record<number, PipelineInfo>;
+}
+
+/**
+ * Location information for the RA from an evaluation log. Line numbers point into the
+ * human-readable log summary.
+ */
+export interface SummarySymbols {
+  predicates: Record<string, PredicateSymbol>;
+}
+
+// Tuple counts for Expr::Expr::getParent#dispred#f0820431#ff@76d6745o:
+const NON_RECURSIVE_TUPLE_COUNT_REGEXP = /^Evaluated relational algebra for predicate (?<predicateName>\S+) with tuple counts:$/;
+// Tuple counts for Expr::Expr::getEnclosingStmt#f0820431#bf@923ddwj9 on iteration 0 running pipeline base:
+const RECURSIVE_TUPLE_COUNT_REGEXP = /^Evaluated relational algebra for predicate (?<predicateName>\S+) on iteration (?<iteration>\d+) running pipeline (?<pipeline>\S+) with tuple counts:$/;
+const RETURN_REGEXP = /^\s*return /;
+
+/**
+ * Parse a human-readable evaluation log summary to find the location of the RA for each pipeline
+ * run.
+ *
+ * TODO: Once we're more certain about the symbol format, we should have the CLI generate this as it
+ * generates the human-readabe summary to avoid having to rely on regular expression matching of the
+ * human-readable text.
+ *
+ * @param summaryPath The path to the summary file.
+ * @param symbolsPath The path to the symbols file to generate.
+ */
+export async function generateSummarySymbolsFile(summaryPath: string, symbolsPath: string): Promise<void> {
+  const symbols = await generateSummarySymbols(summaryPath);
+  await fs.writeFile(symbolsPath, JSON.stringify(symbols));
+}
+
+/**
+ * Parse a human-readable evaluation log summary to find the location of the RA for each pipeline
+ * run.
+ *
+ * @param fileLocation The path to the summary file.
+ * @returns Symbol information for the summary file.
+ */
+async function generateSummarySymbols(summaryPath: string): Promise<SummarySymbols> {
+  const summary = await fs.promises.readFile(summaryPath, { encoding: 'utf-8' });
+  const symbols: SummarySymbols = {
+    predicates: {}
+  };
+
+  const lines = summary.split(/\r?\n/);
+  let lineNumber = 0;
+  while (lineNumber < lines.length) {
+    const startLineNumber = lineNumber;
+    lineNumber++;
+    const startLine = lines[startLineNumber];
+    const nonRecursiveMatch = startLine.match(NON_RECURSIVE_TUPLE_COUNT_REGEXP);
+    let predicateName: string | undefined = undefined;
+    let iteration = 0;
+    if (nonRecursiveMatch) {
+      predicateName = nonRecursiveMatch.groups!.predicateName;
+    } else {
+      const recursiveMatch = startLine.match(RECURSIVE_TUPLE_COUNT_REGEXP);
+      if (recursiveMatch?.groups) {
+        predicateName = recursiveMatch.groups.predicateName;
+        iteration = parseInt(recursiveMatch.groups.iteration);
+      }
+    }
+
+    if (predicateName !== undefined) {
+      const raStartLine = lineNumber;
+      let raEndLine: number | undefined = undefined;
+      while ((lineNumber < lines.length) && (raEndLine === undefined)) {
+        const raLine = lines[lineNumber];
+        const returnMatch = raLine.match(RETURN_REGEXP);
+        if (returnMatch) {
+          raEndLine = lineNumber;
+        }
+        lineNumber++;
+      }
+      if (raEndLine !== undefined) {
+        let symbol = symbols.predicates[predicateName];
+        if (symbol === undefined) {
+          symbol = {
+            iterations: {}
+          };
+          symbols.predicates[predicateName] = symbol;
+        }
+        symbol.iterations[iteration] = {
+          startLine: lineNumber,
+          raStartLine: raStartLine,
+          raEndLine: raEndLine
+        };
+      }
+    }
+  }
+
+  return symbols;
+}

extensions/ql-vscode/src/pure/log-summary-parser.ts

@@ -1,3 +1,5 @@
+import { readJsonlFile } from '../log-insights/jsonl-reader';
+
 // TODO(angelapwen): Only load in necessary information and
 // location in bytes for this log to save memory.
 export interface EvalLogData {
@@ -11,16 +13,11 @@ export interface EvalLogData {
 /**
  * A pure method that parses a string of evaluator log summaries into
  * an array of EvalLogData objects.
- *
  */
-export function parseViewerData(logSummary: string): EvalLogData[] {
+export async function parseViewerData(jsonSummaryPath: string): Promise<EvalLogData[]> {
-  // Remove newline delimiters because summary is in .jsonl format.
-  const jsonSummaryObjects: string[] = logSummary.split(/\r?\n\r?\n/g);
   const viewerData: EvalLogData[] = [];
 
-  for (const obj of jsonSummaryObjects) {
+  await readJsonlFile(jsonSummaryPath, async jsonObj => {
-    const jsonObj = JSON.parse(obj);
-
     // Only convert log items that have an RA and millis field
     if (jsonObj.ra !== undefined && jsonObj.millis !== undefined) {
       const newLogData: EvalLogData = {
@@ -31,6 +28,7 @@ export function parseViewerData(logSummary: string): EvalLogData[] {
       };
       viewerData.push(newLogData);
     }
-  }
+  });
+
   return viewerData;
 }

extensions/ql-vscode/src/pure/messages.ts

@@ -654,7 +654,7 @@ export interface ClearCacheParams {
 /**
  * Parameters to start a new structured log
  */
- export interface StartLogParams {
+export interface StartLogParams {
   /**
    * The dataset for which we want to start a new structured log
    */
@@ -668,7 +668,7 @@ export interface ClearCacheParams {
 /**
  * Parameters to terminate a structured log
  */
- export interface EndLogParams {
+export interface EndLogParams {
   /**
    * The dataset for which we want to terminated the log
    */
@@ -1074,12 +1074,12 @@ export const compileUpgradeSequence = new rpc.RequestType<WithProgressId<Compile
 /**
  * Start a new structured log in the evaluator, terminating the previous one if it exists
  */
- export const startLog = new rpc.RequestType<WithProgressId<StartLogParams>, StartLogResult, void, void>('evaluation/startLog');
+export const startLog = new rpc.RequestType<WithProgressId<StartLogParams>, StartLogResult, void, void>('evaluation/startLog');
 
 /**
  * Terminate a structured log in the evaluator. Is a no-op if we aren't logging to the given location
  */
- export const endLog = new rpc.RequestType<WithProgressId<EndLogParams>, EndLogResult, void, void>('evaluation/endLog');
+export const endLog = new rpc.RequestType<WithProgressId<EndLogParams>, EndLogResult, void, void>('evaluation/endLog');
 
 /**
  * Clear the cache of a dataset

extensions/ql-vscode/src/query-history.ts

@@ -9,6 +9,7 @@ import {
   ProviderResult,
   Range,
   ThemeIcon,
+  TreeDataProvider,
   TreeItem,
   TreeView,
   Uri,
@@ -47,6 +48,7 @@ import { WebviewReveal } from './interface-utils';
 import { EvalLogViewer } from './eval-log-viewer';
 import EvalLogTreeBuilder from './eval-log-tree-builder';
 import { EvalLogData, parseViewerData } from './pure/log-summary-parser';
+import { QueryWithResults } from './run-queries';
 
 /**
  * query-history.ts
@@ -114,7 +116,7 @@ const WORKSPACE_QUERY_HISTORY_FILE = 'workspace-query-history.json';
 /**
  * Tree data provider for the query history view.
  */
-export class HistoryTreeDataProvider extends DisposableObject {
+export class HistoryTreeDataProvider extends DisposableObject implements TreeDataProvider<QueryHistoryInfo> {
   private _sortOrder = SortOrder.DateAsc;
 
   private _onDidChangeTreeData = super.push(new EventEmitter<QueryHistoryInfo | undefined>());
@@ -122,6 +124,10 @@ export class HistoryTreeDataProvider extends DisposableObject {
   readonly onDidChangeTreeData: Event<QueryHistoryInfo | undefined> = this
     ._onDidChangeTreeData.event;
 
+  private _onDidChangeCurrentQueryItem = super.push(new EventEmitter<QueryHistoryInfo | undefined>());
+
+  public readonly onDidChangeCurrentQueryItem = this._onDidChangeCurrentQueryItem.event;
+
   private history: QueryHistoryInfo[] = [];
 
   private failedIconPath: string;
@@ -260,7 +266,10 @@ export class HistoryTreeDataProvider extends DisposableObject {
   }
 
   setCurrentItem(item?: QueryHistoryInfo) {
-    this.current = item;
+    if (item !== this.current) {
+      this.current = item;
+      this._onDidChangeCurrentQueryItem.fire(item);
+    }
   }
 
   remove(item: QueryHistoryInfo) {
@@ -286,7 +295,7 @@ export class HistoryTreeDataProvider extends DisposableObject {
 
   set allHistory(history: QueryHistoryInfo[]) {
     this.history = history;
-    this.current = history[0];
+    this.setCurrentItem(history[0]);
     this.refresh();
   }
 
@@ -313,6 +322,12 @@ export class QueryHistoryManager extends DisposableObject {
   queryHistoryScrubber: Disposable | undefined;
   private queryMetadataStorageLocation;
 
+  private readonly _onDidChangeCurrentQueryItem = super.push(new EventEmitter<QueryHistoryInfo | undefined>());
+  readonly onDidChangeCurrentQueryItem = this._onDidChangeCurrentQueryItem.event;
+
+  private readonly _onDidCompleteQuery = super.push(new EventEmitter<LocalQueryInfo>());
+  readonly onDidCompleteQuery = this._onDidCompleteQuery.event;
+
   constructor(
     private readonly qs: QueryServerClient,
     private readonly dbm: DatabaseManager,
@@ -345,6 +360,11 @@ export class QueryHistoryManager extends DisposableObject {
       canSelectMany: true,
     }));
 
+    // Forward any change of current history item from the tree data.
+    this.push(this.treeDataProvider.onDidChangeCurrentQueryItem((item) => {
+      this._onDidChangeCurrentQueryItem.fire(item);
+    }));
+
     // Lazily update the tree view selection due to limitations of TreeView API (see
     // `updateTreeViewSelectionIfVisible` doc for details)
     this.push(
@@ -537,6 +557,11 @@ export class QueryHistoryManager extends DisposableObject {
     this.registerToRemoteQueriesEvents();
   }
 
+  public completeQuery(info: LocalQueryInfo, results: QueryWithResults): void {
+    info.completeThisQuery(results);
+    this._onDidCompleteQuery.fire(info);
+  }
+
   private getCredentials() {
     return Credentials.initialize(this.ctx);
   }
@@ -926,7 +951,7 @@ export class QueryHistoryManager extends DisposableObject {
     }
 
     // Summary log file doesn't exist.
-    if (finalSingleItem.evalLogLocation && fs.pathExists(finalSingleItem.evalLogLocation)) {
+    if (finalSingleItem.evalLogLocation && await fs.pathExists(finalSingleItem.evalLogLocation)) {
       // If raw log does exist, then the summary log is still being generated.
       this.warnInProgressEvalLogSummary();
     } else {
@@ -950,15 +975,14 @@ export class QueryHistoryManager extends DisposableObject {
       return;
     }
 
-    // TODO(angelapwen): Stream the file in. 
+    // TODO(angelapwen): Stream the file in.
-    void fs.readFile(finalSingleItem.jsonEvalLogSummaryLocation, async (err, buffer) => {
+    try {
-      if (err) {
+      const evalLogData: EvalLogData[] = await parseViewerData(finalSingleItem.jsonEvalLogSummaryLocation);
-        throw new Error(`Could not read evaluator log summary JSON file to generate viewer data at ${finalSingleItem.jsonEvalLogSummaryLocation}.`);
-      }
-      const evalLogData: EvalLogData[] = parseViewerData(buffer.toString());
       const evalLogTreeBuilder = new EvalLogTreeBuilder(finalSingleItem.getQueryName(), evalLogData);
       this.evalLogViewer.updateRoots(await evalLogTreeBuilder.getRoots());
-    });
+    } catch (e) {
+      throw new Error(`Could not read evaluator log summary JSON file to generate viewer data at ${finalSingleItem.jsonEvalLogSummaryLocation}.`);
+    }
   }
 
   async handleCancel(

extensions/ql-vscode/src/query-results.ts

@@ -218,6 +218,7 @@ export class LocalQueryInfo {
   public evalLogLocation: string | undefined;
   public evalLogSummaryLocation: string | undefined;
   public jsonEvalLogSummaryLocation: string | undefined;
+  public evalLogSummarySymbolsLocation: string | undefined;
 
   /**
    * Note that in the {@link slurpQueryHistory} method, we create a FullQueryInfo instance
@@ -282,7 +283,7 @@ export class LocalQueryInfo {
     return !!this.completedQuery;
   }
 
-  completeThisQuery(info: QueryWithResults) {
+  completeThisQuery(info: QueryWithResults): void {
     this.completedQuery = new CompletedQueryInfo(info);
 
     // dispose of the cancellation token source and also ensure the source is not serialized as JSON

extensions/ql-vscode/src/queryserver-client.ts

@@ -271,6 +271,10 @@ export function findJsonQueryEvalLogSummaryFile(resultPath: string): string {
   return path.join(resultPath, 'evaluator-log.summary.jsonl');
 }
 
+export function findQueryEvalLogSummarySymbolsFile(resultPath: string): string {
+  return path.join(resultPath, 'evaluator-log.summary.symbols.json');
+}
+
 export function findQueryEvalLogEndSummaryFile(resultPath: string): string {
   return path.join(resultPath, 'evaluator-log-end.summary');
-}
+}

extensions/ql-vscode/src/run-queries.ts

@@ -37,6 +37,7 @@ import { ensureMetadataIsComplete } from './query-results';
 import { SELECT_QUERY_NAME } from './contextual/locationFinder';
 import { DecodedBqrsChunk } from './pure/bqrs-cli-types';
 import { getErrorMessage } from './pure/helpers-pure';
+import { generateSummarySymbolsFile } from './log-insights/summary-parser';
 
 /**
  * run-queries.ts
@@ -107,6 +108,10 @@ export class QueryEvaluationInfo {
     return qsClient.findJsonQueryEvalLogSummaryFile(this.querySaveDir);
   }
 
+  get evalLogSummarySymbolsPath() {
+    return qsClient.findQueryEvalLogSummarySymbolsFile(this.querySaveDir);
+  }
+
   get evalLogEndSummaryPath() {
     return qsClient.findQueryEvalLogEndSummaryFile(this.querySaveDir);
   }
@@ -206,6 +211,8 @@ export class QueryEvaluationInfo {
           if (config.isCanary()) { // Generate JSON summary for viewer.
             await qs.cliServer.generateJsonLogSummary(this.evalLogPath, this.jsonEvalLogSummaryPath);
             queryInfo.jsonEvalLogSummaryLocation = this.jsonEvalLogSummaryPath;
+            await generateSummarySymbolsFile(this.evalLogSummaryPath, this.evalLogSummarySymbolsPath);
+            queryInfo.evalLogSummarySymbolsLocation = this.evalLogSummarySymbolsPath;
           }
         } else {
           void showAndLogWarningMessage(`Failed to write structured evaluator log to ${this.evalLogPath}.`);
@@ -333,8 +340,8 @@ export class QueryEvaluationInfo {
   }
 
   /**
-   * Calls the appropriate CLI command to generate a human-readable log summary 
+   * Calls the appropriate CLI command to generate a human-readable log summary
-   * and logs to the Query Server console and query log file. 
+   * and logs to the Query Server console and query log file.
    */
   displayHumanReadableLogSummary(queryInfo: LocalQueryInfo, qs: qsClient.QueryServerClient): void {
     queryInfo.evalLogLocation = this.evalLogPath;

extensions/ql-vscode/src/vscode-tests/no-workspace/eval-log-tree-builder.test.ts

@@ -2,108 +2,108 @@ import { expect } from 'chai';
 import EvalLogTreeBuilder from '../../eval-log-tree-builder';
 import { EvalLogData } from '../../pure/log-summary-parser';
 
-describe('EvalLogTreeBuilder', () => {      
+describe('EvalLogTreeBuilder', () => {
-    it('should build the log tree roots', async () => {
+  it('should build the log tree roots', async () => {
-        const evalLogDataItems: EvalLogData[] = [
+    const evalLogDataItems: EvalLogData[] = [
-            {
+      {
-                predicateName: 'quick_eval#query#ffffffff',
+        predicateName: 'quick_eval#query#ffffffff',
-                millis: 1,
+        millis: 1,
-                resultSize: 596,
+        resultSize: 596,
-                ra: { 
+        ra: {
-                    pipeline: [
+          pipeline: [
-                        '{1} r1',
+            '{1} r1',
-                        '{2} r2',
+            '{2} r2',
-                        'return r2'
+            'return r2'
-                    ] 
+          ]
-                },
-            }
-        ];
-    
-        const expectedRoots = [
-            {
-                label: 'test-query.ql',
-                children: undefined
-            }
-        ];
-    
-        const expectedPredicate = [  
-            {
-                label: 'quick_eval#query#ffffffff (596 tuples, 1 ms)',
-                children: undefined,
-                parent: undefined
-            },
-        ];
-    
-        const expectedRA = [  
-            {
-                label: 'Pipeline: pipeline',
-                children: undefined,
-                parent: undefined
-            }
-        ];
-    
-        const expectedPipelineSteps = [{
-            label: '{1} r1',
-            children: [],
-            parent: undefined
         },
-        {
+      }
-            label: '{2} r2',
+    ];
-            children: [],
-            parent: undefined
-        },
-        {
-            label: 'return r2',
-            children: [],
-            parent: undefined
-        }];
 
-        const builder = new EvalLogTreeBuilder('test-query.ql', evalLogDataItems);
+    const expectedRoots = [
-        const roots = await builder.getRoots();
+      {
-        
+        label: 'test-query.ql',
-        // Force children, parent to be undefined for ease of testing. 
+        children: undefined
-        expect(roots.map(
+      }
-            r => ({ ...r, children: undefined })
+    ];
-        )).to.deep.eq(expectedRoots);
 
-        expect((roots[0].children.map(
+    const expectedPredicate = [
-            pred => ({ ...pred, children: undefined, parent: undefined }) 
+      {
-         ))).to.deep.eq(expectedPredicate);
+        label: 'quick_eval#query#ffffffff (596 tuples, 1 ms)',
+        children: undefined,
+        parent: undefined
+      },
+    ];
 
-        expect((roots[0].children[0].children.map(
+    const expectedRA = [
-           ra => ({ ...ra, children: undefined, parent: undefined }) 
+      {
-        ))).to.deep.eq(expectedRA);
+        label: 'Pipeline: pipeline',
-        
+        children: undefined,
-        // Pipeline steps' children should be empty so do not force undefined children here. 
+        parent: undefined
-        expect(roots[0].children[0].children[0].children.map(
+      }
-            step => ({ ...step, parent: undefined }) 
+    ];
-        )).to.deep.eq(expectedPipelineSteps);
-    });
 
-    it('should build the tree with descriptive message when no data exists', async () => {
+    const expectedPipelineSteps = [{
-        // Force children, parent to be undefined for ease of testing. 
+      label: '{1} r1',
-        const expectedRoots = [
+      children: [],
-            {
+      parent: undefined
-                label: 'test-query-cached.ql',
+    },
-                children: undefined
+    {
-            }
+      label: '{2} r2',
-        ];
+      children: [],
-        const expectedNoPredicates = [
+      parent: undefined
-            {
+    },
-                label: 'No predicates evaluated in this query run.',
+    {
-                children: [], // Should be empty so do not force empty here. 
+      label: 'return r2',
-                parent: undefined
+      children: [],
-            }
+      parent: undefined
-        ];
+    }];
-        const builder = new EvalLogTreeBuilder('test-query-cached.ql', []);
-        const roots = await builder.getRoots();
 
-        expect(roots.map(
+    const builder = new EvalLogTreeBuilder('test-query.ql', evalLogDataItems);
-            r => ({ ...r, children: undefined })
+    const roots = await builder.getRoots();
-        )).to.deep.eq(expectedRoots);    
 
-        expect(roots[0].children.map(
+    // Force children, parent to be undefined for ease of testing. 
-            noPreds => ({ ...noPreds, parent: undefined }) 
+    expect(roots.map(
-        )).to.deep.eq(expectedNoPredicates);
+      r => ({ ...r, children: undefined })
-    });
+    )).to.deep.eq(expectedRoots);
-});
+
+    expect((roots[0].children.map(
+      pred => ({ ...pred, children: undefined, parent: undefined })
+    ))).to.deep.eq(expectedPredicate);
+
+    expect((roots[0].children[0].children.map(
+      ra => ({ ...ra, children: undefined, parent: undefined })
+    ))).to.deep.eq(expectedRA);
+
+    // Pipeline steps' children should be empty so do not force undefined children here. 
+    expect(roots[0].children[0].children[0].children.map(
+      step => ({ ...step, parent: undefined })
+    )).to.deep.eq(expectedPipelineSteps);
+  });
+
+  it('should build the tree with descriptive message when no data exists', async () => {
+    // Force children, parent to be undefined for ease of testing. 
+    const expectedRoots = [
+      {
+        label: 'test-query-cached.ql',
+        children: undefined
+      }
+    ];
+    const expectedNoPredicates = [
+      {
+        label: 'No predicates evaluated in this query run.',
+        children: [], // Should be empty so do not force empty here. 
+        parent: undefined
+      }
+    ];
+    const builder = new EvalLogTreeBuilder('test-query-cached.ql', []);
+    const roots = await builder.getRoots();
+
+    expect(roots.map(
+      r => ({ ...r, children: undefined })
+    )).to.deep.eq(expectedRoots);
+
+    expect(roots[0].children.map(
+      noPreds => ({ ...noPreds, parent: undefined })
+    )).to.deep.eq(expectedNoPredicates);
+  });
+});

extensions/ql-vscode/test/pure-tests/log-scanner.test.ts

@@ -0,0 +1,45 @@
+import { expect } from 'chai';
+import 'mocha';
+import { EvaluationLogProblemReporter, EvaluationLogScannerSet } from '../../src/log-insights/log-scanner';
+import { JoinOrderScannerProvider } from '../../src/log-insights/join-order';
+import * as path from 'path';
+
+interface TestProblem {
+  predicateName: string;
+  raHash: string;
+  iteration: number;
+  message: string;
+}
+
+class TestProblemReporter implements EvaluationLogProblemReporter {
+  public readonly problems: TestProblem[] = [];
+
+  public reportProblem(predicateName: string, raHash: string, iteration: number, message: string): void {
+    this.problems.push({
+      predicateName,
+      raHash,
+      iteration,
+      message
+    });
+  }
+
+  public log(message: string): void {
+    console.log(message);
+  }
+}
+
+describe('log scanners', function() {
+  it('should detect bad join orders', async function() {
+    const scanners = new EvaluationLogScannerSet();
+    scanners.registerLogScannerProvider(new JoinOrderScannerProvider());
+    const summaryPath = path.join(__dirname, 'evaluator-log-summaries/bad-join-order.jsonl');
+    const problemReporter = new TestProblemReporter();
+    await scanners.scanLog(summaryPath, problemReporter);
+
+    expect(problemReporter.problems.length).to.equal(1);
+    expect(problemReporter.problems[0].predicateName).to.equal('#select#ff');
+    expect(problemReporter.problems[0].raHash).to.equal('1bb43c97jpmuh8r2v0f9hktim63');
+    expect(problemReporter.problems[0].iteration).to.equal(0);
+    expect(problemReporter.problems[0].message).to.equal('Relation \'#select#ff\' has an inefficient join order. Its join order metric is 4961.83, which is larger than the threshold of 50.00.');
+  });
+});

extensions/ql-vscode/test/pure-tests/log-summary-parser.test.ts

@@ -1,5 +1,4 @@
 import { expect } from 'chai';
-import * as fs from 'fs-extra';
 import * as path from 'path';
 import 'mocha';
 
@@ -8,8 +7,8 @@ import { parseViewerData } from '../../src/pure/log-summary-parser';
 describe('Evaluator log summary tests', async function() {
   describe('for a valid summary text', async function() {
     it('should return only valid EvalLogData objects', async function() {
-      const validSummaryText = await fs.readFile(path.join(__dirname, 'evaluator-log-summaries/valid-summary.jsonl'), 'utf8');
+      const validSummaryPath = path.join(__dirname, 'evaluator-log-summaries/valid-summary.jsonl');
-      const logDataItems = parseViewerData(validSummaryText.toString());
+      const logDataItems = await parseViewerData(validSummaryPath);
       expect(logDataItems).to.not.be.undefined;
       expect(logDataItems.length).to.eq(3);
       for (const item of logDataItems) {
@@ -27,14 +26,14 @@ describe('Evaluator log summary tests', async function() {
     });
 
     it('should not parse a summary header object', async function() {
-      const invalidHeaderText = await fs.readFile(path.join(__dirname, 'evaluator-log-summaries/invalid-header.jsonl'), 'utf8');
+      const invalidHeaderPath = path.join(__dirname, 'evaluator-log-summaries/invalid-header.jsonl');
-      const logDataItems = parseViewerData(invalidHeaderText);
+      const logDataItems = await parseViewerData(invalidHeaderPath);
       expect(logDataItems.length).to.eq(0);
     });
 
     it('should not parse a log event missing RA or millis fields', async function() {
-      const invalidSummaryText = await fs.readFile(path.join(__dirname, 'evaluator-log-summaries/invalid-summary.jsonl'), 'utf8');
+      const invalidSummaryPath = path.join(__dirname, 'evaluator-log-summaries/invalid-summary.jsonl');
-      const logDataItems = parseViewerData(invalidSummaryText);
+      const logDataItems = await parseViewerData(invalidSummaryPath);
       expect(logDataItems.length).to.eq(0);
     });
   });