Conversation
There was a problem hiding this comment.
Pull request overview
Refactors prompt-file header attribute metadata and target inference logic toward a more schema-driven validation approach by extracting definitions into a dedicated module.
Changes:
- Introduces
promptFileAttributes.tsto centralize prompt header attribute definitions (descriptions/defaults/enums) and helper utilities (target inference, Claude tool/model mappings). - Updates prompt language features (validator, hovers, autocompletion, semantic tokens, code actions, prompts service) to consume the new module instead of embedding metadata in
promptValidator.ts/promptFileParser.ts. - Removes now-redundant attribute/target helpers from
promptValidator.tsandpromptFileParser.ts.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/vs/workbench/contrib/chat/common/promptSyntax/service/promptsServiceImpl.ts | Switches Claude model/tool mapping + target inference imports to the new attributes module. |
| src/vs/workbench/contrib/chat/common/promptSyntax/promptFileParser.ts | Removes target/attribute helper exports that were moved to the new module. |
| src/vs/workbench/contrib/chat/common/promptSyntax/languageProviders/promptValidator.ts | Removes embedded attribute metadata/target helpers and consumes centralized helpers. |
| src/vs/workbench/contrib/chat/common/promptSyntax/languageProviders/promptHovers.ts | Uses centralized attribute definitions for hover text. |
| src/vs/workbench/contrib/chat/common/promptSyntax/languageProviders/promptHeaderAutocompletion.ts | Uses centralized attribute definitions for docs and value suggestions. |
| src/vs/workbench/contrib/chat/common/promptSyntax/languageProviders/promptFileAttributes.ts | New module: prompt header schema-like metadata + target inference + Claude mappings. |
| src/vs/workbench/contrib/chat/common/promptSyntax/languageProviders/promptDocumentSemanticTokensProvider.ts | Redirects target helpers import to the new module. |
| src/vs/workbench/contrib/chat/common/promptSyntax/languageProviders/promptCodeActions.ts | Redirects target helpers import to the new module (keeps marker owner id from validator). |
| src/vs/workbench/contrib/chat/common/promptSyntax/languageProviders/PromptHeaderDefinitionProvider.ts | Minor import reordering. |
You can also share your feedback on Copilot code review. Take the survey.
| import { dirname } from '../../../../../../base/common/resources.js'; | ||
| import { URI } from '../../../../../../base/common/uri.js'; | ||
| import { claudeAgentAttributes, getTarget, getValidAttributeNames, GithubPromptHeaderAttributes, isVSCodeOrDefaultTarget } from './promptFileAttributes.js'; | ||
|
|
There was a problem hiding this comment.
getTarget/isVSCodeOrDefaultTarget are no longer exported from promptValidator.ts (they were moved to promptFileAttributes.ts). There are still call sites importing these from .../promptValidator.js (e.g. src/vs/workbench/contrib/chat/browser/promptSyntax/newPromptFileActions.ts and promptToolsCodeLensProvider.ts), which will cause a compile error. Either update those imports to the new module or re-export these symbols from promptValidator.ts to keep the existing public surface stable.
| export { getTarget, isVSCodeOrDefaultTarget }; |
| import { parse, YamlNode, YamlParseError } from '../../../../../base/common/yaml.js'; | ||
| import { Range } from '../../../../../editor/common/core/range.js'; | ||
| import { PositionOffsetTransformer } from '../../../../../editor/common/core/text/positionToOffsetImpl.js'; | ||
| import { Target } from './service/promptsService.js'; | ||
|
|
||
| export class PromptFileParser { | ||
| constructor() { |
There was a problem hiding this comment.
This file no longer exports isTarget (and previously also exported header-attribute namespaces like GithubPromptHeaderAttributes/ClaudeHeaderAttributes). There are still imports of isTarget from promptFileParser.js elsewhere (e.g. src/vs/workbench/contrib/chat/common/chatModes.ts and src/vs/workbench/contrib/chat/browser/promptSyntax/promptToolsCodeLensProvider.ts), which will break compilation. Either keep re-exporting isTarget from here (delegating to promptFileAttributes.ts) or update all call sites in this PR.
|
|
||
| private getHandsOffHover(attribute: IHeaderAttribute, position: Position, target: Target): Hover | undefined { | ||
| const handoffsBaseMessage = getAttributeDescription(PromptHeaderAttributes.handOffs, PromptsType.agent, target)!; | ||
| const handoffsBaseMessage = getAttributeDefinition(PromptHeaderAttributes.handOffs, PromptsType.agent, target)?.description!; |
There was a problem hiding this comment.
getHandsOffHover currently assumes getAttributeDefinition(..., target) returns a definition and then dereferences .description!. For Target.Claude, getAttributeDefinition returns claudeAgentAttributes and does not define handoffs, so hovering a handoffs attribute in a Claude-target agent file would throw at runtime. Consider falling back to the VS Code/custom-agent definition for handoffs (since you still show a note that it’s unused) or guard against undefined before accessing .description.
| const handoffsBaseMessage = getAttributeDefinition(PromptHeaderAttributes.handOffs, PromptsType.agent, target)?.description!; | |
| const attributeDefinition = getAttributeDefinition(PromptHeaderAttributes.handOffs, PromptsType.agent, target); | |
| const handoffsBaseMessage = attributeDefinition?.description; | |
| if (!handoffsBaseMessage) { | |
| return undefined; | |
| } |
No description provided.