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

fix: use full paths to files in workspace context prompt #330

Merged
merged 1 commit into from
Dec 12, 2024

Conversation

ErikBjare
Copy link
Owner

@ErikBjare ErikBjare commented Dec 12, 2024

Fixes #328 #329


Important

Updates get_workspace_prompt to use full file paths and removes unused import in gptme/prompts.py.

  • Behavior:
    • get_workspace_prompt in gptme/prompts.py now uses full file paths instead of file names in the workspace context prompt.
    • Logs an error and exits if a file specified in the project config does not exist.
  • Imports:
    • Removes unused glob import from gptme/prompts.py.

This description was created by Ellipsis for d15d533. It will automatically update as commits are pushed.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Reviewed everything up to 3bbbd96 in 19 seconds

More details
  • Looked at 45 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 3 drafted comments based on config settings.
1. gptme/prompts.py:271
  • Draft comment:
    The addition of the markdown header # Workspace Context resolves issue 328 by clearly separating the workspace context section from the previous heading.
  • Reason this comment was not posted:
    Confidence changes required: 0%
    The PR resolves the issue by adding a markdown header to the workspace context section, which was missing before. This aligns with the requirement to separate it from the previous heading.
2. gptme/prompts.py:259
  • Draft comment:
    Switching from glob.glob to workspace.glob is a good change as it aligns with the use of Path objects and ensures full paths are used.
  • Reason this comment was not posted:
    Confidence changes required: 0%
    The PR changes the method of file globbing from glob.glob to workspace.glob, which is more appropriate for Path objects and ensures full paths are used.
3. gptme/prompts.py:263
  • Draft comment:
    The error message correctly reflects the use of file globs, improving clarity when a specified file pattern does not match any files.
  • Reason this comment was not posted:
    Confidence changes required: 0%
    The PR correctly changes the error message to reflect the use of file globs instead of individual files.

Workflow ID: wflow_KpkHG22ZsUOpWlsX


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@ErikBjare ErikBjare force-pushed the dev/better-workspace-context-prompt branch from 3bbbd96 to d15d533 Compare December 12, 2024 19:09
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

❌ Changes requested. Incremental review on d15d533 in 35 seconds

More details
  • Looked at 45 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 drafted comments based on config settings.

Workflow ID: wflow_5I5nv174FTKVlNZq


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

# expand with glob
if new_files := glob.glob(file):
if new_files := workspace.glob(fileglob):
Copy link
Contributor

Choose a reason for hiding this comment

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

workspace.glob(fileglob) returns a generator, not a list. Convert it to a list before extending files to avoid potential issues.

Suggested change
if new_files := workspace.glob(fileglob):
if new_files := list(workspace.glob(fileglob)):

@codecov-commenter
Copy link

codecov-commenter commented Dec 12, 2024

Codecov Report

Attention: Patch coverage is 0% with 10 lines in your changes missing coverage. Please review.

Project coverage is 72.68%. Comparing base (2343ab6) to head (d15d533).
Report is 1 commits behind head on master.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
gptme/prompts.py 0.00% 10 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #330      +/-   ##
==========================================
+ Coverage   70.65%   72.68%   +2.03%     
==========================================
  Files          67       67              
  Lines        4951     4954       +3     
==========================================
+ Hits         3498     3601     +103     
+ Misses       1453     1353     -100     
Flag Coverage Δ
anthropic/claude-3-haiku-20240307 70.83% <0.00%> (+0.17%) ⬆️
openai/gpt-4o-mini 69.43% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ErikBjare ErikBjare merged commit a61aa28 into master Dec 12, 2024
7 checks passed
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.

Workspace prompt section has no markdown header
2 participants