Skip to content
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

Pass full filepath in prompt to the LLM. #7472

Merged
merged 13 commits into from
Mar 26, 2025

Conversation

thenamankumar
Copy link
Member

@thenamankumar thenamankumar commented Mar 19, 2025

closes: https://linear.app/sourcegraph/issue/CODY-5393/bug-fix-send-the-file-path-to-the-model-for-at-mentions

This PR updates the get text from editor state logic to include the full file display path and not just the file basename.

The text is sent to the LLM as the human message text.

Test plan

unit tests added.

mention a file and debug & test if the full file display path is included in the prompt.

@thenamankumar thenamankumar force-pushed the naman/pass-full-filepath-in-prompt branch from 9c116c8 to ff18990 Compare March 20, 2025 07:59
@thenamankumar thenamankumar marked this pull request as ready for review March 20, 2025 07:59
@thenamankumar thenamankumar requested a review from abeatrix March 20, 2025 08:02
Copy link
Contributor

@abeatrix abeatrix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! I think the test is failing cause of the windows path. There is an helper function that you can use to fix the tests on windows

version: 1,
contextItem: {
type: 'file',
uri: 'file:///foo/bar/file1.go',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
uri: 'file:///foo/bar/file1.go',
uri: testFileUri('/foo/bar/file1.go'),

i think this should fix the test on windows

@thenamankumar thenamankumar force-pushed the naman/pass-full-filepath-in-prompt branch from 69e2fe4 to c28c06a Compare March 24, 2025 05:04
)
})
})

describe('editorStateFromPromptString', () => {
test('converts to rich mentions', async () => {
const input = ps`What are @${PromptString.fromDisplayPath(
URI.file('foo.go')
)}:3-5 and @${PromptString.fromDisplayPath(URI.file('bar.go'))} about?`
testFileUri('foo.go')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
testFileUri('foo.go')
displayPath(testFileUri('/foo.go'))

if it's fromDisplayPath?

@thenamankumar thenamankumar force-pushed the naman/pass-full-filepath-in-prompt branch from 268ea6b to 4a7be80 Compare March 25, 2025 05:11
@thenamankumar thenamankumar merged commit f0390f4 into main Mar 26, 2025
19 of 22 checks passed
@thenamankumar thenamankumar deleted the naman/pass-full-filepath-in-prompt branch March 26, 2025 09:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants