prompt files: only show problems for files open in editor#304872
prompt files: only show problems for files open in editor#304872
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates chat prompt-file diagnostics so that prompt validation markers are only produced for prompt files that are currently open in an editor, reducing Problems-view noise from non-visible prompt models.
Changes:
- Move prompt language feature registration and prompt validation contribution from
common/intobrowser/to use editor-tracking APIs. - Replace model-service-wide prompt validation with tracking based on open
ICodeEditorinstances and reference counting per model. - Update chat workbench contribution wiring to reference the new browser contribution entrypoint.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/vs/workbench/contrib/chat/common/promptSyntax/promptFileContributions.ts | Removed old common-layer workbench contribution (language features + validator contribution). |
| src/vs/workbench/contrib/chat/common/promptSyntax/languageProviders/promptValidator.ts | Removed the old model-service-based validator contribution/tracker; keeps core PromptValidator + marker owner id. |
| src/vs/workbench/contrib/chat/browser/promptSyntax/promptFileContributions.ts | New browser-layer contribution that registers language features and tracks open editors to emit/clear prompt markers. |
| src/vs/workbench/contrib/chat/browser/chat.contribution.ts | Updated import to the new browser-layer PromptLanguageFeaturesProvider. |
Comments suppressed due to low confidence (1)
src/vs/workbench/contrib/chat/browser/promptSyntax/promptFileContributions.ts:130
- The
onDidChangeModelLanguagehandler unconditionally callsrelease(model.uri)before re-acquiring. If a model transitions from a non-prompt language to a prompt language while it is open in multiple editors, each editor will fire this event; the second (and later) handlers can end up decrementing/discarding the tracker created by the first handler, leaving an incorrectrefCountand potentially removing markers while an editor is still open. Consider usinge.oldLanguage/e.newLanguageto decide whether to release/acquire (only release if the old language was a prompt type, only acquire if the new language is a prompt type), and ensure prompt-type changes recreate/update the existing tracker accordingly.
this.localDisposables.add(editor.onDidChangeModelLanguage((e) => {
const model = editor.getModel();
if (model) {
release(model.uri);
acquire(editor);
| this.localDisposables.add(editor.onDidChangeModel((e) => { | ||
| if (e.oldModelUrl) { | ||
| release(e.oldModelUrl); | ||
| } | ||
| acquire(editor); |
There was a problem hiding this comment.
The per-editor listeners created in onCodeEditorAdd are registered into localDisposables, but nothing disposes/removes those listener disposables when the corresponding editor is removed. Over time (opening/closing many editors), this DisposableStore can grow without bound. Consider tracking a per-editor DisposableStore (e.g. in a map keyed by editor.getId()), disposing it in onCodeEditorRemove, and only keeping the global event subscriptions in localDisposables.
| // Track models from editors that are currently open | ||
| for (const editor of this.codeEditorService.listCodeEditors()) { | ||
| acquire(editor); | ||
| } | ||
|
|
||
| // When an editor is added, start tracking its model | ||
| this.localDisposables.add(this.codeEditorService.onCodeEditorAdd((editor: ICodeEditor) => { | ||
| acquire(editor); |
There was a problem hiding this comment.
updateRegistration() calls listCodeEditors() and runs acquire(editor) for already-existing editors, but it only wires up editor.onDidChangeModel / onDidChangeModelLanguage listeners inside onCodeEditorAdd. This means editors that already exist when this contribution is created won't update tracking when their model/language changes, so markers can remain for files that are no longer open. Consider registering the per-editor listeners for the initial listCodeEditors() set as well (e.g. via a shared registerEditor(editor) helper used by both the initial loop and onCodeEditorAdd).
This issue also appears on line 126 of the same file.
…304872) * prompt files: only show problems for files open in editor * update
Fixes #304778