Skip to content

Commit

Permalink
fix: show warning if function description length is incorrect (#12155)
Browse files Browse the repository at this point in the history 
* fix: show warning if function description length incorrect

* fix: show warning if function description length incorrect

test: ut

test: ut

test: ut

test: ut

test: ut

test: ut

test: ut

test: ut

* test: ut

* test: ut

* refactor: string feedback

* refactor: string feedback
  • Loading branch information
@yuqizhou77
yuqizhou77 authored Aug 6, 2024
1 parent 238346b commit ae956ed
Show file tree
Hide file tree
Showing 8 changed files with 346 additions and 50 deletions.
4 changes: 4 additions & 0 deletions packages/fx-core/resource/package.nls.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,10 @@
"core.copilotPlugin.scaffold.summary.warning.teamsManifest.missingCardTemlate.mitigation": " Mitigation: create Adaptive Card template in \"%s\" and then update \"%s\" field to the relative path in \"%s\".",
"core.copilotPlugin.scaffold.summary.warning.api.optionalParametersOnly": "There is no required parameter defined in API \"%s\". The first optional parameter is set as the parameter for command \"%s\".",
"core.copilotPlugin.scaffold.summary.warning.api.optionalParametersOnly.mitigation": " Mitigation: If \"%s\" is not what you need, edit the parameter of command \"%s\" in \"%s\". The parameter name should match what's defined in \"%s\".",
"core.copilotPlugin.scaffold.summary.warning.pluginManifest.missingFunctionDescription": "Description for function \"%s\" is missing.",
"core.copilotPlugin.scaffold.summary.warning.pluginManifest.missingFunctionDescription.mitigation": " Mitigation: Update description for \"%s\" in \"%s\"",
"core.copilotPlugin.scaffold.summary.warning.pluginManifest.functionDescription.lengthExceeding": "Description for function \"%s\" shortened to %s characters to meet the length requirement.",
"core.copilotPlugin.scaffold.summary.warning.pluginManifest.functionDescription.lengthExceeding.mitigation": " Mitigation: Update description for \"%s\" in \"%s\" so that Copilot can trigger the function.",
"core.createCapabilityQuestion.titleNew": "Capabilities",
"core.createCapabilityQuestion.placeholder": "Select a capability",
"core.createProjectQuestion.option.description.preview": "Preview",
Expand Down
13 changes: 9 additions & 4 deletions packages/fx-core/src/component/generator/apiSpec/generator.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -530,9 +530,10 @@ export class SpecGenerator extends DefaultTemplateGenerator {
await fs.ensureDir(apiSpecFolderPath);

let generateResult;
let pluginManifestPath: string | undefined;

if (getTemplateInfosState.isPlugin) {
const pluginManifestPath = path.join(
pluginManifestPath = path.join(
destinationPath,
AppPackageFolderName,
defaultPluginManifestFileName
Expand Down Expand Up @@ -615,14 +616,18 @@ export class SpecGenerator extends DefaultTemplateGenerator {

// log warnings
if (inputs.platform === Platform.CLI || inputs.platform === Platform.VS) {
const warnSummary = generateScaffoldingSummary(
const warnSummary = await generateScaffoldingSummary(
warnings,
teamsManifest,
path.relative(destinationPath, openapiSpecPath)
path.relative(destinationPath, openapiSpecPath),
pluginManifestPath === undefined
? undefined
: path.relative(destinationPath, pluginManifestPath),
destinationPath
);

if (warnSummary) {
void context.logProvider.info(warnSummary);
context.logProvider.info(warnSummary);
}
}

Expand Down
101 changes: 96 additions & 5 deletions packages/fx-core/src/component/generator/apiSpec/helper.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import {
AppPackageFolderName,
Context,
FxError,
IMessagingExtensionCommand,
Inputs,
ManifestTemplateFileName,
ManifestUtil,
Expand Down Expand Up @@ -56,6 +55,7 @@ import {
import { SummaryConstant } from "../../configManager/constant";
import { manifestUtils } from "../../driver/teamsApp/utils/ManifestUtils";
import { pluginManifestUtils } from "../../driver/teamsApp/utils/PluginManifestUtils";
import { sendTelemetryErrorEvent } from "../../../common/telemetry";

const enum telemetryProperties {
validationStatus = "validation-status",
Expand All @@ -68,11 +68,13 @@ const enum telemetryProperties {
oauth2AuthCount = "oauth2-auth-count",
otherAuthCount = "other-auth-count",
isFromAddingApi = "is-from-adding-api",
failedReason = "failed-reason",
}

const enum telemetryEvents {
validateApiSpec = "validate-api-spec",
listApis = "spec-parser-list-apis-result",
failedToGetGenerateWarning = "failed-to-get-generate-warning",
}

export const copilotPluginParserOptions: ParseOptions = {
Expand Down Expand Up @@ -429,11 +431,22 @@ export function logValidationResults(
void context.logProvider.info(outputMessage);
}

export function generateScaffoldingSummary(
/**
* Generate scaffolding warning summary.
* @param warnings warnings returned from spec-parser.
* @param teamsManifest Teams manifest.
* @param apiSpecFilePath API spec path relative of project path.
* @param pluginManifestPath Plugin manifest path relative of project path.
* @param projectPath Project path.
* @returns Warning message.
*/
export async function generateScaffoldingSummary(
warnings: Warning[],
teamsManifest: TeamsAppManifest,
apiSpecFilePath: string
): string {
apiSpecFilePath: string,
pluginManifestPath: string | undefined,
projectPath: string
): Promise<string> {
const apiSpecWarningMessage = formatApiSpecValidationWarningMessage(
warnings,
apiSpecFilePath,
Expand All @@ -444,7 +457,23 @@ export function generateScaffoldingSummary(
return `${SummaryConstant.NotExecuted} ${warn}`;
});

if (apiSpecWarningMessage.length || manifestWarningMessage.length) {
let pluginWarningMessage: string[] = [];
if (pluginManifestPath) {
const pluginManifestWarningResult = await validatePluginManifestLength(
pluginManifestPath,
projectPath,
warnings
);
pluginWarningMessage = pluginManifestWarningResult.map((warn) => {
return `${SummaryConstant.NotExecuted} ${warn}`;
});
}

if (
apiSpecWarningMessage.length ||
manifestWarningMessage.length ||
pluginWarningMessage.length
) {
let details = "";
if (apiSpecWarningMessage.length) {
details += EOL + apiSpecWarningMessage.join(EOL);
Expand All @@ -454,6 +483,10 @@ export function generateScaffoldingSummary(
details += EOL + manifestWarningMessage.join(EOL);
}

if (pluginWarningMessage.length) {
details += EOL + pluginWarningMessage.join(EOL);
}

return getLocalizedString("core.copilotPlugin.scaffold.summary", details);
} else {
return "";
Expand Down Expand Up @@ -600,6 +633,64 @@ function validateTeamsManifestLength(
return resultWarnings;
}

async function validatePluginManifestLength(
pluginManifestPath: string,
projectPath: string,
warnings: Warning[]
): Promise<string[]> {
const functionDescriptionLimit = 100;
const resultWarnings: string[] = [];

const manifestRes = await pluginManifestUtils.readPluginManifestFile(
path.join(projectPath, pluginManifestPath)
);
if (manifestRes.isErr()) {
sendTelemetryErrorEvent(
"spec-generator",
telemetryEvents.failedToGetGenerateWarning,
manifestRes.error
);
return [];
}

// validate function description
const functions = manifestRes.value.functions;
const functionDescriptionWarnings = warnings
.filter((w) => w.type === WarningType.FuncDescriptionTooLong)
.map((w) => w.data);
if (functions) {
functions.forEach((func) => {
if (!func.description) {
resultWarnings.push(
getLocalizedString(
"core.copilotPlugin.scaffold.summary.warning.pluginManifest.missingFunctionDescription",
func.name
) +
getLocalizedString(
"core.copilotPlugin.scaffold.summary.warning.pluginManifest.missingFunctionDescription.mitigation",
func.name,
pluginManifestPath
)
);
} else if (functionDescriptionWarnings.includes(func.name)) {
resultWarnings.push(
getLocalizedString(
"core.copilotPlugin.scaffold.summary.warning.pluginManifest.functionDescription.lengthExceeding",
func.name,
functionDescriptionLimit
) +
getLocalizedString(
"core.copilotPlugin.scaffold.summary.warning.pluginManifest.functionDescription.lengthExceeding.mitigation",
func.name,
pluginManifestPath
)
);
}
});
}
return resultWarnings;
}

function formatLengthExceedingErrorMessage(field: string, limit: number): string {
return (
getLocalizedString(
Expand Down
16 changes: 11 additions & 5 deletions packages/fx-core/src/core/FxCore.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1692,6 +1692,7 @@ export class FxCore {
}

let generateResult;
let pluginPath: string | undefined;
if (!isPlugin) {
generateResult = await specParser.generate(
manifestPath,
Expand All @@ -1707,6 +1708,7 @@ export class FxCore {
if (pluginPathRes.isErr()) {
return err(pluginPathRes.error);
}
pluginPath = pluginPathRes.value;
generateResult = await specParser.generateForCopilot(
manifestPath,
operations,
Expand All @@ -1725,10 +1727,12 @@ export class FxCore {
});

if (generateResult.warnings && generateResult.warnings.length > 0) {
const warnSummary = generateScaffoldingSummary(
const warnSummary = await generateScaffoldingSummary(
generateResult.warnings,
manifestRes.value,
path.relative(inputs.projectPath!, outputApiSpecPath)
path.relative(inputs.projectPath!, outputApiSpecPath),
pluginPath === undefined ? undefined : path.relative(inputs.projectPath!, pluginPath),
inputs.projectPath!
);

if (warnSummary) {
Expand Down Expand Up @@ -1937,12 +1941,14 @@ export class FxCore {
});

if (generateResult.warnings && generateResult.warnings.length > 0) {
const warnSummary = generateScaffoldingSummary(
const warnSummary = await generateScaffoldingSummary(
generateResult.warnings,
manifestRes.value,
path.relative(inputs.projectPath, openApiSpecFilePath)
path.relative(inputs.projectPath, openApiSpecFilePath),
path.relative(inputs.projectPath, pluginManifestFilePath),
inputs.projectPath
);
context.logProvider.info(warnSummary);
context.logProvider.info(warnSummary + "\n");
}
} catch (e) {
let error: FxError;
Expand Down
Loading

0 comments on commit ae956ed

Please sign in to comment.