Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add prompt links diagnostics provider #241783

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ import { ILanguageService } from '../../../../../../editor/common/languages/lang
import { FileKind, IFileService } from '../../../../../../platform/files/common/files.js';
import { IMenuService, MenuId } from '../../../../../../platform/actions/common/actions.js';
import { getCleanPromptName } from '../../../../../../platform/prompts/common/constants.js';
import { ChatPromptAttachmentModel } from '../../chatAttachmentModel/chatPromptAttachmentModel.js';
import { IContextKeyService } from '../../../../../../platform/contextkey/common/contextkey.js';
import { ChatPromptAttachmentModel } from '../../chatAttachmentModel/chatPromptAttachmentModel.js';
import { IContextMenuService } from '../../../../../../platform/contextview/browser/contextView.js';
import { getDefaultHoverDelegate } from '../../../../../../base/browser/ui/hover/hoverDelegateFactory.js';
import { getFlatContextMenuActions } from '../../../../../../platform/actions/browser/menuEntryActionViewItem.js';
Expand Down Expand Up @@ -118,18 +118,18 @@ export class PromptAttachmentWidget extends Disposable {
// add the issue details in the hover title for the attachment, one
// error/warning at a time because there is a limited space available
if (topError) {
const { isRootError, message: details } = topError;
const isWarning = !isRootError;
const { errorSubject: subject } = topError;
const isError = (subject === 'root');

this.domNode.classList.add(
(isWarning) ? 'warning' : 'error',
(isError) ? 'error' : 'warning',
);

const errorCaption = (isWarning)
? localize('warning', "Warning")
: localize('error', "Error");
const severity = (isError)
? localize('error', "Error")
: localize('warning', "Warning");

title += `\n-\n[${errorCaption}]: ${details}`;
title += `\n[${severity}]: ${topError.localizedMessage}`;
}

const fileWithoutExtension = getCleanPromptName(file);
Expand Down
1 change: 1 addition & 0 deletions src/vs/workbench/contrib/chat/browser/chat.contribution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import { ILanguageModelStatsService, LanguageModelStatsService } from '../common
import { ILanguageModelToolsService } from '../common/languageModelToolsService.js';
import '../common/promptSyntax/languageFeatures/promptLinkProvider.js';
import '../common/promptSyntax/languageFeatures/promptPathAutocompletion.js';
import '../common/promptSyntax/languageFeatures/promptLinkDiagnosticsProvider.js';
import { PromptsService } from '../common/promptSyntax/service/promptsService.js';
import { IPromptsService } from '../common/promptSyntax/service/types.js';
import './promptSyntax/contributions/usePromptCommand.js';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ import { assert } from '../../../../../../../../base/common/assert.js';
import { VSBuffer } from '../../../../../../../../base/common/buffer.js';
import { dirname } from '../../../../../../../../base/common/resources.js';
import { IFileService } from '../../../../../../../../platform/files/common/files.js';
import { isPromptFile, PROMPT_FILE_EXTENSION } from '../../../../../../../../platform/prompts/common/constants.js';
import { ICommandService } from '../../../../../../../../platform/commands/common/commands.js';
import { isPromptFile, PROMPT_FILE_EXTENSION } from '../../../../../../../../platform/prompts/common/constants.js';

/**
* Options for the {@link createPromptFile} utility.
Expand Down
82 changes: 63 additions & 19 deletions src/vs/workbench/contrib/chat/common/promptFileReferenceErrors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@
*--------------------------------------------------------------------------------------------*/

import { URI } from '../../../../base/common/uri.js';
import { basename } from '../../../../base/common/path.js';
import { assert, assertNever } from '../../../../base/common/assert.js';

/**
* Base prompt parsing error class.
*/
export abstract class ParseError extends Error {
abstract class ParseError extends Error {
/**
* Error type name.
*/
Expand Down Expand Up @@ -41,36 +43,36 @@ export abstract class ParseError extends Error {
}

/**
* A generic error for failing to resolve prompt contents stream.
* Base resolve error class used when file reference resolution fails.
*/
export class FailedToResolveContentsStream extends ParseError {
public override errorType = 'FailedToResolveContentsStream';
export abstract class ResolveError extends ParseError {
public abstract override errorType: string;

constructor(
public readonly uri: URI,
public readonly originalError: unknown,
message: string = `Failed to resolve prompt contents stream for '${uri.toString()}': ${originalError}.`,
message?: string,
options?: ErrorOptions,
) {
super(message);
super(message, options);
}
}


/**
* Base resolve error class used when file reference resolution fails.
* A generic error for failing to resolve prompt contents stream.
*/
export abstract class ResolveError extends ParseError {
public abstract override errorType: string;
export class FailedToResolveContentsStream extends ResolveError {
public override errorType = 'FailedToResolveContentsStream';

constructor(
public readonly uri: URI,
message?: string,
options?: ErrorOptions,
uri: URI,
public readonly originalError: unknown,
message: string = `Failed to resolve prompt contents stream for '${uri.toString()}': ${originalError}.`,
) {
super(message, options);
super(uri, message);
}
}


/**
* Error that reflects the case when attempt to open target file fails.
*/
Expand All @@ -89,6 +91,12 @@ export class OpenFailed extends FailedToResolveContentsStream {
}
}

/**
* Character use to join filenames/paths in a chain of references that
* lead to recursion.
*/
const DEFAULT_RECURSIVE_PATH_JOIN_CHAR = ' -> ';

/**
* Error that reflects the case when attempt resolve nested file
* references failes due to a recursive reference, e.g.,
Expand All @@ -106,23 +114,53 @@ export class OpenFailed extends FailedToResolveContentsStream {
export class RecursiveReference extends ResolveError {
public override errorType = 'RecursiveReferenceError';

/**
* Default string representation of the recursive path.
*/
public readonly recursivePathString: string;

constructor(
uri: URI,
public readonly recursivePath: string[],
) {
const references = recursivePath.join(' -> ');
// sanity check - a recursive path must always have at least
// two items in the list, otherwise it is not a recursive loop
assert(
recursivePath.length >= 2,
`Recursive path must contain at least two paths, got '${recursivePath.length}'.`,
);

const pathString = recursivePath.join(DEFAULT_RECURSIVE_PATH_JOIN_CHAR);
super(
uri,
`Recursive references found: ${references}.`,
`Recursive references found: ${pathString}.`,
);
this.recursivePathString = pathString;
}

/**
* Returns a string representation of the recursive path.
*/
public get recursivePathString(): string {
return this.recursivePath.join(' -> ');
public getRecursivePathString(
filename: 'basename' | 'fullpath',
pathJoinCharacter: string = DEFAULT_RECURSIVE_PATH_JOIN_CHAR,
): string {
return this.recursivePath
.map((path) => {
if (filename === 'fullpath') {
return `'${path}'`;
}

if (filename === 'basename') {
return `'${basename(path)}'`;
}

assertNever(
filename,
`Unknown filename format '${filename}'.`,
);
})
.join(pathJoinCharacter);
}

/**
Expand All @@ -138,6 +176,12 @@ export class RecursiveReference extends ResolveError {
return false;
}

// performance optimization - if the paths lengths don't match,
// no need to compare entire strings as they must be different
if (this.recursivePathString.length !== other.recursivePathString.length) {
return false;
}

return this.recursivePathString === other.recursivePathString;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { CancellationError } from '../../../../../../base/common/errors.js';
import { PromptContentsProviderBase } from './promptContentsProviderBase.js';
import { VSBufferReadableStream } from '../../../../../../base/common/buffer.js';
import { CancellationToken } from '../../../../../../base/common/cancellation.js';
import { OpenFailed, NotPromptFile, ParseError, FolderReference } from '../../promptFileReferenceErrors.js';
import { OpenFailed, NotPromptFile, ResolveError, FolderReference } from '../../promptFileReferenceErrors.js';
import { FileChangesEvent, FileChangeType, IFileService } from '../../../../../../platform/files/common/files.js';

/**
Expand Down Expand Up @@ -81,7 +81,7 @@ export class FilePromptContentProvider extends PromptContentsProviderBase<FileCh

fileStream = await this.fileService.readFileStream(this.uri);
} catch (error) {
if (error instanceof ParseError) {
if (error instanceof ResolveError) {
throw error;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,14 @@ import { VSBufferReadableStream } from '../../../../../../base/common/buffer.js'
import { CancellationToken } from '../../../../../../base/common/cancellation.js';
import { isPromptFile } from '../../../../../../platform/prompts/common/constants.js';
import { ObservableDisposable } from '../../../../../../base/common/observableDisposable.js';
import { FailedToResolveContentsStream, ParseError } from '../../promptFileReferenceErrors.js';
import { FailedToResolveContentsStream, ResolveError } from '../../promptFileReferenceErrors.js';
import { cancelPreviousCalls } from '../../../../../../base/common/decorators/cancelPreviousCalls.js';

/**
* Base class for prompt contents providers. Classes that extend this one are responsible to:
*
* - implement the {@linkcode getContentsStream} method to provide the contents stream
* of a prompt; this method should throw a `ParseError` or its derivative if the contents
* of a prompt; this method should throw a `ResolveError` or its derivative if the contents
* cannot be parsed for any reason
* - fire a {@linkcode TChangeEvent} event on the {@linkcode onChangeEmitter} event when
* prompt contents change
Expand Down Expand Up @@ -49,7 +49,7 @@ export abstract class PromptContentsProviderBase<

/**
* Function to get contents stream for the provider. This function should
* throw a `ParseError` or its derivative if the contents cannot be parsed.
* throw a `ResolveError` or its derivative if the contents cannot be parsed.
*
* @param changesEvent The event that triggered the change. The special
* `'full'` value means that everything has changed hence entire prompt
Expand All @@ -75,12 +75,12 @@ export abstract class PromptContentsProviderBase<
* Event emitter for the prompt contents change event.
* See {@linkcode onContentChanged} for more details.
*/
private readonly onContentChangedEmitter = this._register(new Emitter<VSBufferReadableStream | ParseError>());
private readonly onContentChangedEmitter = this._register(new Emitter<VSBufferReadableStream | ResolveError>());

/**
* Event that fires when the prompt contents change. The event is either
* a `VSBufferReadableStream` stream with changed contents or an instance of
* the `ParseError` class representing a parsing failure case.
* the `ResolveError` class representing a parsing failure case.
*
* `Note!` this field is meant to be used by the external consumers of the prompt
* contents provider that the classes that extend this abstract class.
Expand Down Expand Up @@ -112,7 +112,7 @@ export abstract class PromptContentsProviderBase<
this.onContentChangedEmitter.fire(stream);
})
.catch((error) => {
if (error instanceof ParseError) {
if (error instanceof ResolveError) {
this.onContentChangedEmitter.fire(error);

return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*--------------------------------------------------------------------------------------------*/

import { URI } from '../../../../../../base/common/uri.js';
import { ParseError } from '../../promptFileReferenceErrors.js';
import { ResolveError } from '../../promptFileReferenceErrors.js';
import { IDisposable } from '../../../../../../base/common/lifecycle.js';
import { VSBufferReadableStream } from '../../../../../../base/common/buffer.js';

Expand All @@ -27,9 +27,9 @@ export interface IPromptContentsProvider extends IDisposable {
/**
* Event that fires when the prompt contents change. The event is either a
* {@linkcode VSBufferReadableStream} stream with changed contents or
* an instance of the {@linkcode ParseError} error.
* an instance of the {@linkcode ResolveError} error.
*/
onContentChanged(
callback: (streamOrError: VSBufferReadableStream | ParseError) => void,
callback: (streamOrError: VSBufferReadableStream | ResolveError) => void,
): IDisposable;
}
Loading
Loading