-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Upstream model generated file and line linkification #1803
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,96 @@ | ||
| /*--------------------------------------------------------------------------------------------- | ||
| * Copyright (c) Microsoft Corporation. All rights reserved. | ||
| * Licensed under the MIT License. See License.txt in the project root for license information. | ||
| *--------------------------------------------------------------------------------------------*/ | ||
|
|
||
| import { IFileSystemService } from '../../../platform/filesystem/common/fileSystemService'; | ||
| import { FileType } from '../../../platform/filesystem/common/fileTypes'; | ||
| import { IWorkspaceService } from '../../../platform/workspace/common/workspaceService'; | ||
| import { CancellationToken } from '../../../util/vs/base/common/cancellation'; | ||
| import { Location, Position, Range, Uri } from '../../../vscodeTypes'; | ||
| import { coalesceParts, LinkifiedPart, LinkifiedText, LinkifyLocationAnchor } from './linkifiedText'; | ||
| import { IContributedLinkifier, LinkifierContext } from './linkifyService'; | ||
|
|
||
| // Matches markdown links where the text is a path and optional #L anchor is present | ||
| // Example: [src/file.ts](src/file.ts#L10-12) or [src/file.ts](src/file.ts) | ||
| const modelLinkRe = /\[(?<text>[^\]\n]+)\]\((?<target>[^\s)]+)\)/gu; | ||
|
|
||
| export class ModelFilePathLinkifier implements IContributedLinkifier { | ||
| constructor( | ||
| @IFileSystemService private readonly fileSystem: IFileSystemService, | ||
| @IWorkspaceService private readonly workspaceService: IWorkspaceService, | ||
| ) { } | ||
|
|
||
| async linkify(text: string, context: LinkifierContext, token: CancellationToken): Promise<LinkifiedText | undefined> { | ||
| let lastIndex = 0; | ||
| const out: Array<LinkifiedPart | Promise<LinkifiedPart>> = []; | ||
|
|
||
| for (const match of text.matchAll(modelLinkRe)) { | ||
| const prefix = text.slice(lastIndex, match.index); | ||
| if (prefix) { | ||
| out.push(prefix); | ||
| } | ||
| lastIndex = match.index + match[0].length; | ||
|
|
||
| const rawText = match.groups?.['text'] ?? ''; | ||
| const rawTarget = match.groups?.['target'] ?? ''; | ||
|
|
||
| const hashIndex = rawTarget.indexOf('#'); | ||
| const baseTarget = hashIndex === -1 ? rawTarget : rawTarget.slice(0, hashIndex); | ||
| const anchor = hashIndex === -1 ? undefined : rawTarget.slice(hashIndex + 1); | ||
|
|
||
| let decodedBase = baseTarget; | ||
| try { decodedBase = decodeURIComponent(baseTarget); } catch { } | ||
|
|
||
| if (decodedBase !== rawText) { | ||
| out.push(match[0]); | ||
| continue; | ||
| } | ||
|
|
||
| const workspaceFolders = this.workspaceService.getWorkspaceFolders(); | ||
| let resolved: Uri | undefined; | ||
| for (const folder of workspaceFolders) { | ||
| const candidate = Uri.joinPath(folder, decodedBase); | ||
| const stat = await this.tryStat(candidate); | ||
| if (stat) { resolved = stat; break; } | ||
| } | ||
| if (!resolved) { | ||
| out.push(match[0]); | ||
| continue; | ||
| } | ||
|
|
||
| if (anchor && /^L\d+(?:-\d+)?$/.test(anchor)) { | ||
| const m = /^L(\d+)(?:-(\d+))?$/.exec(anchor); | ||
| if (m) { | ||
| const start = parseInt(m[1], 10) - 1; | ||
| const end = (m[2] ? parseInt(m[2], 10) : parseInt(m[1], 10)) - 1; | ||
| if (start >= 0 && end >= start) { | ||
| try { console.log('[linkify][model] linkified range', { path: decodedBase, anchor, requestId: context.requestId }); } catch { } | ||
| out.push(new LinkifyLocationAnchor(new Location(resolved, new Range(new Position(start, 0), new Position(end, 0))))); | ||
| continue; | ||
| } | ||
| } | ||
| } | ||
| try { console.log('[linkify][model] linkified file', { path: decodedBase, requestId: context.requestId }); } catch { } | ||
| out.push(new LinkifyLocationAnchor(resolved)); | ||
| } | ||
|
|
||
| const suffix = text.slice(lastIndex); | ||
| if (suffix) { out.push(suffix); } | ||
|
|
||
| if (!out.length) { | ||
| return undefined; | ||
| } | ||
| return { parts: coalesceParts(await Promise.all(out)) }; | ||
| } | ||
|
|
||
| private async tryStat(uri: Uri): Promise<Uri | undefined> { | ||
| try { | ||
| const stat = await this.fileSystem.stat(uri); | ||
| if (stat.type === FileType.Directory) { | ||
| return uri.path.endsWith('/') ? uri : uri.with({ path: uri.path + '/' }); | ||
| } | ||
| return uri; | ||
| } catch { return undefined; } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| /*--------------------------------------------------------------------------------------------- | ||
| * Copyright (c) Microsoft Corporation. All rights reserved. | ||
| * Licensed under the MIT License. See License.txt in the project root for license information. | ||
| *--------------------------------------------------------------------------------------------*/ | ||
|
|
||
| import { suite, test } from 'vitest'; | ||
| import { Location, Position, Range } from '../../../../vscodeTypes'; | ||
| import { LinkifyLocationAnchor } from '../../common/linkifiedText'; | ||
| import { assertPartsEqual, createTestLinkifierService, linkify, workspaceFile } from './util'; | ||
|
|
||
| suite('Model File Path Linkifier', () => { | ||
| test('Should linkify model generated file references with line range', async () => { | ||
| const service = createTestLinkifierService('src/file.ts'); | ||
| const result = await linkify(service, '[src/file.ts](src/file.ts#L10-12)'); | ||
| const anchor = result.parts[0] as LinkifyLocationAnchor; | ||
| const expected = new LinkifyLocationAnchor(new Location(workspaceFile('src/file.ts'), new Range(new Position(9, 0), new Position(11, 0)))); | ||
| assertPartsEqual([anchor], [expected]); | ||
| }); | ||
|
|
||
| test('Should linkify single line anchors', async () => { | ||
| const service = createTestLinkifierService('src/file.ts'); | ||
| const result = await linkify(service, '[src/file.ts](src/file.ts#L5)'); | ||
| const anchor = result.parts[0] as LinkifyLocationAnchor; | ||
| const expected = new LinkifyLocationAnchor(new Location(workspaceFile('src/file.ts'), new Range(new Position(4, 0), new Position(4, 0)))); | ||
| assertPartsEqual([anchor], [expected]); | ||
| }); | ||
|
|
||
| test('Should fallback when text does not match base path', async () => { | ||
| const service = createTestLinkifierService('src/file.ts'); | ||
| const result = await linkify(service, '[other](src/file.ts#L2-3)'); | ||
| assertPartsEqual(result.parts, ['[other](src/file.ts#L2-3)']); | ||
| }); | ||
|
|
||
| test('Should fallback for invalid anchor syntax', async () => { | ||
| const service = createTestLinkifierService('src/file.ts'); | ||
| const result = await linkify(service, '[src/file.ts](src/file.ts#Lines10-12)'); | ||
| assertPartsEqual(result.parts, ['[src/file.ts](src/file.ts#Lines10-12)']); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| /*--------------------------------------------------------------------------------------------- | ||
| * Copyright (c) Microsoft Corporation. All rights reserved. | ||
| * Licensed under the MIT License. See License.txt in the project root for license information. | ||
| *--------------------------------------------------------------------------------------------*/ | ||
|
|
||
| import { PromptElement } from '@vscode/prompt-tsx'; | ||
| import { Tag } from '../base/tag'; | ||
|
|
||
| export class FileLinkificationInstructions extends PromptElement<{}> { | ||
| render() { | ||
| return <Tag name='file_linkification'> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just a gut feeling but this seems pretty verbose and I'm not sure how well models will follow the good/bad method written this way |
||
| ALWAYS convert any filename or path mention into a markdown link using one of these canonical forms (1-based line numbers):<br /> | ||
| `[path/to/file.ts](path/to/file.ts)` whole file<br /> | ||
| `[path/to/file.ts](path/to/file.ts#L10)` single line<br /> | ||
| `[path/to/file.ts](path/to/file.ts#L10-12)` inclusive line range<br /> | ||
| Transformation examples (apply this rewriting proactively):<br /> | ||
| Bad: `The main function is in exampleScript.ts at line 25.`<br /> | ||
| Good: `The main function is in [exampleScript.ts](exampleScript.ts#L25).`<br /> | ||
| Bad: `See src/utils/math.ts lines 40-44 for the loop.`<br /> | ||
| Good: `See [src/utils/math.ts](src/utils/math.ts#L40-44) for the loop.`<br /> | ||
| Bad: `Config lives in docs/My File.md`<br /> | ||
| Good: `Config lives in [docs/My File.md](docs/My%20File.md)`<br /> | ||
| Rules (enforced):<br /> | ||
| - Bracket text MUST exactly equal the file path portion before any `#` anchor (omit line hash from text).<br /> | ||
| Wrong: `[src/file.ts#L10](src/file.ts#L10)` (anchor included inside brackets)<br /> | ||
| Correct: `[src/file.ts](src/file.ts#L10)` (anchor only in link target)<br /> | ||
| - Use workspace-relative POSIX paths (forward slashes). Do NOT invent paths; only cite existing ones or ones already shown in context. If uncertain about directory, prefer reading context before guessing.<br /> | ||
| - If you state a line or range in prose, IMMEDIATELY integrate it into the link anchor instead of leaving it separate.<br /> | ||
| - Only add an anchor when certain; if unsure about exact lines, emit the whole-file link (no anchor) and optionally gather more context before citing lines.<br /> | ||
| - For multiple disjoint ranges from one file, emit separate links (one per range).<br /> | ||
| - Do NOT wrap these links themselves in backticks; they are plain markdown links (the code fences/backticks rule applies only to ordinary inline path references, not these links).<br /> | ||
| - Percent-encode spaces ONLY in the target; bracket text remains unencoded (e.g. `[docs/My File.md](docs/My%20File.md)`).<br /> | ||
| - Prefer citing a range (`#L10-12`) if you reference ≥2 consecutive lines; otherwise single line anchor (`#L10`).<br /> | ||
| - Never leave a bare filename like `exampleScript.ts` in prose without converting it to a link unless you are explicitly quoting user input you will transform next.<br /> | ||
| - Backticks vs links: Backtick wrapping applies ONLY to ordinary inline path mentions you are not converting into links. When producing a markdown link (`[path](path[#Lx[-y]])`), do NOT wrap the link itself in backticks; the link replaces the backticked form.<br /> | ||
| Self-correction: If you output a filename without the required link format, immediately correct yourself in the very next message by restating it with the proper link.<br /> | ||
| Goal: Maximize model-emitted links so fallback legacy linkifier rarely triggers.<br /> | ||
| </Tag>; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,6 +14,7 @@ import { EXISTING_CODE_MARKER } from '../panel/codeBlockFormattingRules'; | |
| import { MathIntegrationRules } from '../panel/editorIntegrationRules'; | ||
| import { KeepGoingReminder } from './agentPrompt'; | ||
| import { ApplyPatchInstructions, CodesearchModeInstructions, DefaultAgentPromptProps, detectToolCapabilities, GenericEditingTips, McpToolInstructions, NotebookInstructions } from './defaultAgentInstructions'; | ||
| import { FileLinkificationInstructions } from './fileLinkificationInstructions'; | ||
| import { IAgentPrompt, PromptConstructor, PromptRegistry } from './promptRegistry'; | ||
|
|
||
| export class DefaultOpenAIAgentPrompt extends PromptElement<DefaultAgentPromptProps> { | ||
|
|
@@ -101,6 +102,7 @@ export class DefaultOpenAIAgentPrompt extends PromptElement<DefaultAgentPromptPr | |
| <NotebookInstructions {...this.props} /> | ||
| <Tag name='outputFormatting'> | ||
| Use proper Markdown formatting in your answers. When referring to a filename or symbol in the user's workspace, wrap it in backticks.<br /> | ||
| <FileLinkificationInstructions /> | ||
| <Tag name='example'> | ||
| The class `Person` is in `src/models/person.ts`.<br /> | ||
| The function `calculateTotal` is defined in `lib/utils/math.ts`.<br /> | ||
|
|
@@ -313,6 +315,7 @@ class DefaultGpt5AgentPrompt extends PromptElement<DefaultAgentPromptProps> { | |
| For casual greetings, acknowledgements, or other one-off conversational messages that are not delivering substantive information or structured results, respond naturally without section headers or bullet formatting.<br /> | ||
| <br /> | ||
| When referring to a filename or symbol in the user's workspace, wrap it in backticks.<br /> | ||
| <FileLinkificationInstructions /> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think this should break up the example |
||
| <Tag name='example'> | ||
| The class `Person` is in `src/models/person.ts`. | ||
| </Tag> | ||
|
|
@@ -394,10 +397,14 @@ class CodexStyleGPT5CodexPrompt extends PromptElement<DefaultAgentPromptProps> { | |
| * Accepted: absolute, workspace-relative, a/ or b/ diff prefixes, or bare filename/suffix.<br /> | ||
| * Do not use URIs like file://, vscode://, or https://.<br /> | ||
| * Examples: src/app.ts, C:\repo\project\main.rs<br /> | ||
| <br /> | ||
| <FileLinkificationInstructions /> | ||
| </InstructionMessage>; | ||
| } | ||
| } | ||
|
|
||
| // FileLinkificationInstructions extracted to shared fileLinkificationInstructions.tsx | ||
|
|
||
| class OpenAIPromptResolver implements IAgentPrompt { | ||
|
|
||
| static readonly familyPrefixes = ['gpt', 'o4-mini', 'o3-mini', 'OpenAI']; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's break up the existing
FilePathLinkifierinto the real links functionality and the inline code functionality. Maybe just delete the real links stuff fromFilePathLinkifierand keep this class. I'd like to avoid the duplication though and make it so we just have one place that handles the markdown file links