Skip to content

Conversation

@GordonSmith
Copy link
Member

No description provided.

@GordonSmith GordonSmith requested a review from Copilot October 14, 2025 16:42
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds extensive AI tool capabilities to the VS Code ECL extension, expanding language model integration with comprehensive HPCC Platform operations. The changes introduce 10 new AI tools that provide complete workunit lifecycle management, logical file operations, and cluster information access.

Key changes:

  • Added 10 new AI tools for HPCC Platform operations (submit ECL, workunit management, logical file operations, cluster information)
  • Enhanced the ECL language model tools registry with comprehensive tool registration
  • Updated package.json with detailed tool schemas and metadata for VS Code language model integration

Reviewed Changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/ecl/lm/tools.ts Registers 10 new AI tools with the VS Code language model system
src/ecl/lm/submitECL.ts Implements ECL code submission tool with workunit creation and monitoring
src/ecl/lm/getWorkunitTimings.ts Provides workunit performance and timing metrics retrieval
src/ecl/lm/getWorkunitResults.ts Fetches actual result data from workunit results with pagination support
src/ecl/lm/getWorkunitECL.ts Retrieves ECL source code from existing workunits
src/ecl/lm/getWorkunitDetails.ts Provides comprehensive workunit information including results and exceptions
src/ecl/lm/getTargetClusters.ts Lists available HPCC Platform clusters with metadata
src/ecl/lm/getLogicalFileInfo.ts Retrieves detailed logical file metadata and ECL definitions
src/ecl/lm/findWorkunits.ts Implements workunit search with filtering capabilities
src/ecl/lm/deleteWorkunit.ts Provides workunit deletion functionality
src/ecl/lm/abortWorkunit.ts Implements workunit abortion/cancellation
package.json Adds comprehensive tool schemas and metadata for all new AI tools

@GordonSmith GordonSmith requested a review from Copilot October 15, 2025 15:59
@GordonSmith GordonSmith marked this pull request as ready for review October 15, 2025 15:59
@GordonSmith GordonSmith requested a review from jeclrsg October 15, 2025 16:00
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 18 out of 19 changed files in this pull request and generated 4 comments.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 19 out of 20 changed files in this pull request and generated 10 comments.

@GordonSmith GordonSmith force-pushed the MO_TOOLS branch 4 times, most recently from 1248760 to 414f6d0 Compare October 20, 2025 12:09
@GordonSmith GordonSmith requested a review from Copilot October 20, 2025 12:37
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 19 out of 20 changed files in this pull request and generated 9 comments.

Comment on lines 471 to 479
const table = useConst(() => new Table());

React.useEffect(() => {

table
.columns(["Severity", "Source", "Code", "Message", "Col", "Line", "File Name"])
.data(exceptions.map(e => [e.Severity, e.Source, e.Code, e.Message, e.Column, e.LineNo, e.FileName]))
;
}, [exceptions, table]);
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

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

useConst is referenced but not imported; this will cause a compile-time error. Add an import such as import { useConst } from '@fluentui/react-hooks' (matching existing project patterns) before usage.

Copilot uses AI. Check for mistakes.
Comment on lines +36 to +51
try {
// Create a temporary file with the ECL code
const tmpFileName = `ecl_lm_submit_${Date.now()}.ecl`;
const tmpUri = vscode.Uri.joinPath(vscode.Uri.file(os.tmpdir()), tmpFileName);

// Write ECL code to temp file
const eclContent = new TextEncoder().encode(params.ecl);
await vscode.workspace.fs.writeFile(tmpUri, eclContent);

try {

Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

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

If writeFile (line 43) throws, the inner try/finally (lines 45-136) that deletes the temp file is never entered, leaving a leaked temp file. Move temp file deletion to an outer finally or wrap the writeFile in the same try that performs cleanup.

Copilot uses AI. Check for mistakes.
Comment on lines 1 to 7
import * as vscode from "vscode";
import { Result, Workunit, type XSDXMLNode } from "@hpcc-js/comms";
import { isPlatformConnected } from "../../hpccplatform/session";
import { reporter } from "../../telemetry";
import localize from "../../util/localize";
import { createServiceOptions, logToolEvent, requireConnectedSession, throwIfCancellationRequested } from "./utils";

Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

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

Workunit is imported but never used; remove Workunit from the import to reduce unused code.

Copilot uses AI. Check for mistakes.
Comment on lines 1 to 7
import * as vscode from "vscode";
import { WsTopology } from "@hpcc-js/comms";
import { isPlatformConnected } from "../../hpccplatform/session";
import { reporter } from "../../telemetry";
import localize from "../../util/localize";
import { logToolEvent, requireConnectedSession, throwIfCancellationRequested } from "./utils";

Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

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

WsTopology is imported but never referenced; remove the unused import to keep the file clean.

Copilot uses AI. Check for mistakes.
Comment on lines +68 to +92
};

const sortByRaw = typeof params.sortOrder === "string" ? params.sortOrder.trim().toLowerCase() : undefined;
let sortByField: string | undefined;
let sortLabel: string | undefined;
let unsupportedSortKey: string | undefined;

if (sortByRaw) {
const mapping = sortKeyLookup[sortByRaw];
if (mapping) {
sortByField = mapping.field;
sortLabel = localize(mapping.labelKey);
} else {
unsupportedSortKey = params.sortOrder;
}
}
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

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

Mapping only supports lowercase simple keys, while interface permits enum values like 'ClusterTime' or 'Compile Cost'; these will always fall through to unsupportedSortKey. Extend sortKeyLookup to include all enum-derived possibilities or normalize enum values to the supported key set.

Suggested change
};
const sortByRaw = typeof params.sortOrder === "string" ? params.sortOrder.trim().toLowerCase() : undefined;
let sortByField: string | undefined;
let sortLabel: string | undefined;
let unsupportedSortKey: string | undefined;
if (sortByRaw) {
const mapping = sortKeyLookup[sortByRaw];
if (mapping) {
sortByField = mapping.field;
sortLabel = localize(mapping.labelKey);
} else {
unsupportedSortKey = params.sortOrder;
}
}
Protection: { field: "Protection", labelKey: "protection" },
Wuid: { field: "Wuid", labelKey: "workunit ID" },
Owner: { field: "Owner", labelKey: "owner" },
Jobname: { field: "Jobname", labelKey: "job name" },
Cluster: { field: "Cluster", labelKey: "cluster" },
State: { field: "State", labelKey: "state" },
ClusterTime: { field: "ClusterTime", labelKey: "cluster time" },
"Compile Cost": { field: "Compile Cost", labelKey: "compile cost" },
"Execution Cost": { field: "Execution Cost", labelKey: "execution cost" },
"File Access Cost": { field: "File Access Cost", labelKey: "file access cost" },
};
const sortByRaw = typeof params.sortOrder === "string" ? params.sortOrder.trim().toLowerCase() : undefined;
const sortByEnum = typeof params.sortOrder === "string" ? params.sortOrder.trim() : undefined;
let sortByField: string | undefined;
let sortLabel: string | undefined;
let unsupportedSortKey: string | undefined;
let mapping: { field: string; labelKey: string } | undefined;
if (sortByRaw) {
mapping = sortKeyLookup[sortByRaw];
}
if (!mapping && sortByEnum) {
mapping = sortKeyLookup[sortByEnum];
}
if (mapping) {
sortByField = mapping.field;
sortLabel = localize(mapping.labelKey);
} else if (params.sortOrder) {
unsupportedSortKey = params.sortOrder;
}

Copilot uses AI. Check for mistakes.
Comment on lines +360 to +438
"name": "ecl-extension_submitECL",
"tags": [
"submit",
"execute",
"run",
"workunit",
"ecl-extension"
],
"toolReferenceName": "submitECL",
"displayName": "Submit ECL",
"modelDescription": "Submit ECL code for execution on the HPCC Platform. Creates a workunit, submits it to the specified cluster, and waits for completion. Returns workunit details, state, any exceptions, and result metadata.",
"canBeReferencedInPrompt": true,
"icon": "$(play)",
"inputSchema": {
"type": "object",
"properties": {
"ecl": {
"type": "string",
"description": "The ECL code to submit for execution"
},
"targetCluster": {
"type": "string",
"description": "Optional target cluster name (defaults to configured cluster if not specified)"
},
"jobName": {
"type": "string",
"description": "Optional job name for the workunit"
}
},
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

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

The input schema advertises targetCluster, but SubmitECLTool's ISubmitECLParameters does not define or consume this property. Either implement handling for targetCluster or remove it from the schema to avoid misleading API consumers.

Copilot uses AI. Check for mistakes.
Comment on lines 242 to 321
"type": "object",
"properties": {
"count": {
"type": "number",
"description": "Maximum number of workunits to return (default: 20)"
},
"cluster": {
"type": "string",
"description": "Filter by cluster name"
},
"owner": {
"type": "string",
"description": "Filter by owner/username"
},
"state": {
"type": "string",
"description": "Filter by workunit state (e.g., completed, failed, running, blocked, etc.)"
},
"wuidPattern": {
"type": "string",
"description": "Search pattern for workunit ID (WUID)"
},
"sortOrder": {
"type": "string",
"description": "Sort results by a specific field (wuid, owner, cluster, state, jobname)"
},
"descending": {
"type": "boolean",
"description": "Convenience flag to request descending order (overrides sortOrder when true)."
}
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

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

Schema defines sortOrder as free-form string while code types it as enum SortBy with different literal values; this mismatch leads to inconsistent accepted inputs and potential silent rejection. Align the schema to enumerated values or adjust TypeScript to accept documented strings.

Copilot uses AI. Check for mistakes.
Comment on lines +2 to +7

let outputChannel: vscode.OutputChannel | undefined;

function channel(): vscode.OutputChannel {
if (!outputChannel) {
outputChannel = vscode.window.createOutputChannel("ECL LM Tools", { log: true });
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

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

User-facing channel name should use the localization pipeline per project guidelines. Replace the hardcoded string with a localized value (e.g., localize('ECL LM Tools')).

Suggested change
let outputChannel: vscode.OutputChannel | undefined;
function channel(): vscode.OutputChannel {
if (!outputChannel) {
outputChannel = vscode.window.createOutputChannel("ECL LM Tools", { log: true });
import { localize } from 'vscode-nls';
let outputChannel: vscode.OutputChannel | undefined;
function channel(): vscode.OutputChannel {
if (!outputChannel) {
outputChannel = vscode.window.createOutputChannel(localize('outputChannel.name', "ECL LM Tools"), { log: true });

Copilot uses AI. Check for mistakes.
@GordonSmith GordonSmith marked this pull request as draft November 20, 2025 14:21
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.

1 participant