Show file validation indicators in Chat Customizations editor#308340
Show file validation indicators in Chat Customizations editor#308340joshspicer wants to merge 2 commits intomainfrom
Conversation
Screenshot ChangesBase: Changed (40) |
There was a problem hiding this comment.
Pull request overview
This PR improves the Chat Customizations management editor by surfacing validation/discovery failures directly in the customization list, instead of silently hiding invalid files, and by overlaying prompt-diagnostics marker severity onto list items.
Changes:
- Add
skipReasonToStatus()to map prompt discovery skip reasons to localized status + tooltip text. - Include skipped/errored discovery results for agents/skills/prompts/instructions (and add discovery-derived status for hook files).
- Inject
IMarkerServiceto overlay prompt-diagnostics marker severity onto items and refresh when markers change.
Show a summary per file
| File | Description |
|---|---|
src/vs/workbench/contrib/chat/browser/aiCustomization/aiCustomizationListWidget.ts |
Adds discovery-skip status indicators, overlays prompt validation markers onto list item status, and refreshes the list on marker changes. |
Copilot's findings
Comments suppressed due to low confidence (2)
src/vs/workbench/contrib/chat/browser/aiCustomization/aiCustomizationListWidget.ts:1323
fetchItemsForSectionalways callsoverlayMarkerStatus(items). This means any caller (includingcomputeItemCountForSection, which runs for multiple sections) will synchronously read markers for every item. Consider making marker overlay optional (only for the currently displayed section) or moving it out of the shared fetch path so section-count computation doesn’t pay the marker-read cost.
private async fetchItemsForSection(section: AICustomizationManagementSection): Promise<IAICustomizationListItem[]> {
const promptType = sectionToPromptType(section);
const activeDescriptor = this.harnessService.getActiveDescriptor();
let items: IAICustomizationListItem[];
if (activeDescriptor.itemProvider && promptType) {
items = await this.fetchProviderItemsForSection(activeDescriptor, promptType);
} else {
items = await this.fetchCoreItemsForSection(promptType);
}
return this.overlayMarkerStatus(items);
}
src/vs/workbench/contrib/chat/browser/aiCustomization/aiCustomizationListWidget.ts:1352
overlayMarkerStatusreads markers by callingmarkerService.read({ resource: uri, owner })once per item URI. With many items this becomes O(n) reads per refresh and is amplified by frequent refresh triggers. You can reduce overhead by doing a singlemarkerService.read({ owner: MARKERS_OWNER_ID })and then grouping/filtering those markers by URI (using the item URI set), or by usingtaketo avoid reading all markers when you only need severity/count.
private overlayMarkerStatus(items: IAICustomizationListItem[]): IAICustomizationListItem[] {
// Collect unique URIs to query markers for
const uris = new ResourceSet(items.map(i => i.uri));
const markersByUri = new ResourceMap<{ severity: MarkerSeverity; message: string }>();
for (const uri of uris) {
const markers = this.markerService.read({ resource: uri, owner: MARKERS_OWNER_ID });
if (markers.length > 0) {
// Find worst severity
let worstSeverity = MarkerSeverity.Hint;
let worstMessage = '';
for (const m of markers) {
if (m.severity > worstSeverity) {
worstSeverity = m.severity;
worstMessage = m.message;
}
}
const summary = markers.length === 1
? worstMessage
: localize('markerCount', "{0} problems found in this file", markers.length);
markersByUri.set(uri, { severity: worstSeverity, message: summary });
}
}
- Files reviewed: 1/1 changed files
- Comments generated: 3
defe5e4 to
8038456
Compare
There was a problem hiding this comment.
Pull request overview
This PR improves the Chat Customizations management editor by surfacing validation/discovery failures directly in the list (instead of hiding broken files or showing them without context), using existing per-item status icon rendering.
Changes:
- Adds
skipReasonToStatus()to map discovery skip reasons to list item status + localized tooltip text. - Extends core-item fetching to include discovery-skipped files across prompt types (agents/skills/prompts/instructions; partial integration for hooks).
- Overlays prompt diagnostics from
IMarkerService(MARKERS_OWNER_ID) onto list items and refreshes the list on marker changes.
Show a summary per file
| File | Description |
|---|---|
| src/vs/workbench/contrib/chat/browser/aiCustomization/aiCustomizationListWidget.ts | Adds discovery-skip status mapping, includes skipped files in list results, overlays marker-based validation status, and refreshes on marker updates. |
Copilot's findings
Comments suppressed due to low confidence (2)
src/vs/workbench/contrib/chat/browser/aiCustomization/aiCustomizationListWidget.ts:1658
- In the Hooks section, skipped/errored files shown via
fileStatusare not markeddisabled: true(they keepdisabled: disabledUris.has(hookFile.uri)). This means discovery-skipped hook files (e.g. parse errors, all-hooks-disabled) won’t appear greyed out and won’t be treated as disabled in when-clause filtering, which is inconsistent with the other prompt types where discovery-skipped files are added asdisabled: true. Consider settingdisabled: truewhenfileStatusis present (or otherwise reflecting discovery-skipped state) so the UI consistently indicates that the file wasn’t loaded.
const discoveryResult = discoveryByUri.get(hookFile.uri);
const fileStatus = discoveryResult?.status === 'skipped' && discoveryResult.skipReason !== 'disabled'
? skipReasonToStatus(discoveryResult)
: undefined;
items.push({
id: hookFile.uri.toString(),
uri: hookFile.uri,
name: hookFile.name || this.getFriendlyName(filename),
filename,
storage: hookFile.storage,
promptType,
disabled: disabledUris.has(hookFile.uri),
status: fileStatus?.status,
statusMessage: fileStatus?.statusMessage,
src/vs/workbench/contrib/chat/browser/aiCustomization/aiCustomizationListWidget.ts:1593
- In the Hooks path,
discoveryByUriis only used when parsing fails /parsedHooksstays false. For skip reasons likeclaude-hooks-disabled(and potentially others) parsing can succeed and the widget will render individual hook entries with no disabled styling or status indicator, even though discovery reported the file as skipped. Consider applying discovery status to hook entries (or to the file) even when hooks were parsed, so the list accurately reflects that the file is currently skipped/disabled by configuration.
// Build a lookup from discovery info for per-file status (parse errors, disabled hooks, etc.)
const hookDiscovery = await this.promptsService.getDiscoveryInfo(PromptsType.hook, CancellationToken.None);
const discoveryByUri = new ResourceMap<IPromptFileDiscoveryResult>();
for (const file of hookDiscovery.files) {
discoveryByUri.set(file.promptPath.uri, file);
}
- Files reviewed: 1/1 changed files
- Comments generated: 1
…ditor Customization files that fail during discovery (bad YAML frontmatter, invalid JSON hooks, missing required fields, duplicate names, etc.) were previously either silently hidden from the management editor list or shown without any error indication. This change pipes the existing IPromptFileDiscoveryResult status through to the list widget's IAICustomizationListItem, reusing the existing status icon rendering (warning/error codicons with hover tooltip) that was previously only used for remote/provider items. For each prompt type (agents, skills, prompts, hooks, instructions), skipped files from getDiscoveryInfo() are now included in the list with: - status: 'error' or 'degraded' based on the skip reason severity - statusMessage: a human-readable explanation of the problem - disabled: true (greyed out) The skipReasonToStatus() helper maps each PromptFileSkipReason to the appropriate severity and localized message. Files explicitly disabled by the user (skipReason === 'disabled') are excluded since they already appear via the existing disabled-file path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Query the marker service for prompt-diagnostics markers (from the existing PromptValidator) on each customization item's URI. When a file has validation markers (e.g. from being open in an editor with frontmatter errors), the list item's status is upgraded to 'degraded' (warning) or 'error' based on the worst marker severity. This composites with discovery-level status from the previous ifcommit both discovery and markers report issues, the worst severity wins. Also subscribes to IMarkerService.onMarkerChanged so the list reactively updates when markers change (e.g. user fixes a validation error in an open editor). The subscription is filtered to only refresh when changed URIs match items currently displayed in the list. Key implementation details: - overlayMarkerStatus() is called from fetchItemsForSection() and applies to both core and provider code paths - Markers are filtered by MARKERS_OWNER_ID to only consider prompt-file diagnostics (not unrelated linter markers) - For files with multiple markers, shows a count summary; for single markers, shows the message directly Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
8038456 to
526b7c0
Compare
Summary
Customization files (agents, skills, hooks, prompts, instructions) that fail during discovery — bad YAML frontmatter, invalid JSON hooks, missing required fields, duplicate names, etc. — were previously either silently hidden from the management editor list or shown without any error indication. This PR surfaces file validation issues directly in the Chat Customizations editor.
Changes
1. Surface discovery status as validation indicators
skipReasonToStatus()helper that mapsPromptFileSkipReason→{status, statusMessage}with localized messagesfetchCoreItemsForSection()for all 5 prompt types to callgetDiscoveryInfo()and include skipped/errored files in the listdisabled: true) with warning/error codicon + hover tooltip2. Overlay IMarkerService diagnostics
IMarkerServiceinto the list widget and addsoverlayMarkerStatus()post-processing stepMARKERS_OWNER_ID(prompt diagnostics only)onMarkerChangedfor reactive updates when validation markers change in open editorsWhat users see
Notes
hookFileSchema+HOOK_SCHEMA_URIinchat.contribution.ts) — no additional schema work neededskipReason === 'disabled'case is excluded since user-disabled files already appear via the existing disabled-file path