Fix implicit context variables for notebook cells (microsoft#285858)

* Fix implicit context variables for notebook cells

* Address comments

Author: DonJayamanne

src/vs/workbench/contrib/chat/browser/attachments/chatImplicitContext.ts

@@ -29,6 +29,8 @@ import { ILanguageModelIgnoredFilesService } from '../../common/ignoredFiles.js'
 import { getPromptsTypeForLanguageId } from '../../common/promptSyntax/promptTypes.js';
 import { IChatWidget, IChatWidgetService } from '../chat.js';
 import { IChatContextService } from '../contextContrib/chatContextService.js';
+import { ITextModel } from '../../../../../editor/common/model.js';
+import { IRange } from '../../../../../editor/common/core/range.js';
 
 export class ChatImplicitContextContribution extends Disposable implements IWorkbenchContribution {
 	static readonly ID = 'chat.implicitContext';
@@ -210,18 +212,22 @@ export class ChatImplicitContextContribution extends Disposable implements IWork
 				const selection = codeEditor?.getSelection();
 				const visibleRanges = codeEditor?.getVisibleRanges() || [];
 				newValue = activeCell.uri;
-				if (isEqual(codeEditor?.getModel()?.uri, activeCell.uri)) {
+				const cellModel = codeEditor?.getModel();
+				if (cellModel && isEqual(cellModel.uri, activeCell.uri)) {
 					if (selection && !selection.isEmpty()) {
 						newValue = { uri: activeCell.uri, range: selection } satisfies Location;
 						isSelection = true;
 					} else if (visibleRanges.length > 0) {
-						// Merge visible ranges. Maybe the reference value could actually be an array of Locations?
-						// Something like a Location with an array of Ranges?
-						let range = visibleRanges[0];
-						visibleRanges.slice(1).forEach(r => {
-							range = range.plusRange(r);
-						});
-						newValue = { uri: activeCell.uri, range } satisfies Location;
+						// If the entire cell is visible, just use the cell URI, no need to specify range.
+						if (!isEntireCellVisible(cellModel, visibleRanges)) {
+							// Merge visible ranges. Maybe the reference value could actually be an array of Locations?
+							// Something like a Location with an array of Ranges?
+							let range = visibleRanges[0];
+							visibleRanges.slice(1).forEach(r => {
+								range = range.plusRange(r);
+							});
+							newValue = { uri: activeCell.uri, range } satisfies Location;
+						}
 					}
 				}
 			} else {
@@ -267,6 +273,13 @@ export class ChatImplicitContextContribution extends Disposable implements IWork
 	}
 }
 
+function isEntireCellVisible(cellModel: ITextModel, visibleRanges: IRange[]): boolean {
+	if (visibleRanges.length === 1 && visibleRanges[0].startLineNumber === 1 && visibleRanges[0].startColumn === 1 && visibleRanges[0].endLineNumber === cellModel.getLineCount() && visibleRanges[0].endColumn === cellModel.getLineMaxColumn(visibleRanges[0].endLineNumber)) {
+		return true;
+	}
+	return false;
+}
+
 export class ChatImplicitContext extends Disposable implements IChatRequestImplicitVariableEntry {
 	get id() {
 		if (URI.isUri(this.value)) {

src/vs/workbench/contrib/chat/browser/attachments/implicitContextAttachment.ts

@@ -14,7 +14,7 @@ import { Disposable, DisposableStore } from '../../../../../base/common/lifecycl
 import { Schemas } from '../../../../../base/common/network.js';
 import { basename, dirname } from '../../../../../base/common/resources.js';
 import { URI } from '../../../../../base/common/uri.js';
-import { Location } from '../../../../../editor/common/languages.js';
+import { isLocation, Location } from '../../../../../editor/common/languages.js';
 import { ILanguageService } from '../../../../../editor/common/languages/language.js';
 import { IModelService } from '../../../../../editor/common/services/model.js';
 import { localize } from '../../../../../nls.js';
@@ -214,7 +214,11 @@ export class ImplicitContextAttachmentWidget extends Disposable {
 			this.attachmentModel.addContext(context);
 		} else {
 			const file = URI.isUri(this.attachment.value) ? this.attachment.value : this.attachment.value.uri;
-			this.attachmentModel.addFile(file);
+			if (file.scheme === Schemas.vscodeNotebookCell && isLocation(this.attachment.value)) {
+				this.attachmentModel.addFile(file, this.attachment.value.range);
+			} else {
+				this.attachmentModel.addFile(file);
+			}
 		}
 		this.widgetRef()?.focusInput();
 	}