-
Notifications
You must be signed in to change notification settings - Fork 323
feat(context): Use XML tags to delimit context chunks #863
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: master
Are you sure you want to change the base?
Conversation
@munen Showing the basename is intentional. We assumed that the user is fine with sharing the file contents with the LLM since they explicitly added it, but that they might not want to expose the full path. The full path might have sensitive components. It's not clear from the context interface that the absolute path of the file will be sent to the LLM as well. What are your thoughts on this? |
Thank you for getting back to me quickly 🙏 Before having tools, I agree with you. Then, there also was no good reason to add the path, so it was wasted tokens. Now, with tools, I see these things in my interactions often:
Apart from the UX issue, I'd argue that file contents should be generally more sensitive than the path. If we're talking about sensitive information, then the main question is: "What is the attack vector?". In my understanding it's giving sensitive data which is then used for training. Giving away API secrets, for example, is probably not smart, because should it be reproduced, it can be used by anyone. However, giving away a path, even should it be reproduced by the llm, the attacker wouldn't know who has this path and how to access it. So I don't see the attack vector, personally. If you do see security issues that I'm not seeing, do you see a different tactic of having tools that allow for file system modification? Could it be configuration to include the full path? |
On a related note regarding a different kind of security ("How can I sandbox gptel tools?") I have found a solution that works great for me and have documented it on our blog: https://200ok.ch/posts/2025-05-23_sandboxing_ai_tools:_how_guix_containers_keep_your_host_safe_while_empowering_llms.html There's also a bit of gptel praise sprinkled in. Thank you kindly for continuously putting effort into building this amazing tool. Using it makes my day on a regular basis 🙇 |
As a side note, I used Claude 4 to help me with the blog post. It was a great example of why I’ve created the above PR. I added the blog post (Org mode) to the gptel-context and added the project root directory to the prompt. I even explicitly stated that it already had access to the file in the context. However, it consistently kept using the read_file tool. So in effect, the blog post has been twice in the context, plus I have a wasted tool call. Only when I gave it the full path as part of the prompt, it understood that it had already the file in the context. In effect, the current context implementation was of no help, unfortunately. It would have been less work for both me and the llm had I only given it the file path in the prompt(; Of course, that’s not a solution since the gptel context generally gives me way more power over what context to provide and to constantly keep it updated. In fact, I believe the gptel context to be a prime USP compared to other llm tools. Comparatively, even with the same llm, I get better results in gptel than in other tools. I believe that the main difference is that with gptel I can granularly select the relevant context. That focus in turn produces superior results. |
That makes sense. I'm assuming from your description that in your branch (where you include the full path instead of basenames), most LLMs you've tried recognize that the file is already available and avoid calling the
It's not a security issue, just a matter of UX and obeying user expectations. If I add There are a couple of minor changes that might help reduce this expectation gap.
We can do both, but 2 will override 1 in most cases anyway. Understanding whether this works well requires testing, so you'll have to let me know. |
The blog led me to your gptel setup -- you have many interesting LLM tools. Except for I've never managed the kind of free-form agentic use of gptel that you see in other coding assistants/agents. gptel lacks high-quality LLM tools and the right prompts for that kind of usage, both of which were probably derived from a lot of testing.
I'm very glad to hear that! |
Yes.
Based on initial tests, option 1 would be best. To be honest, when using upstream gptel (without the PR), I do exactly that. There's also no need for the username whatsoever. It's wasted tokens at best(; With my sandboxing approach, and without the current PR, I'll usually start my prompts with something like: project dir: /workspace (taken from my last vibecoded pr). I'll refactor the PR to use option 1 and do some testing and will get back to you with results.
Some of the tools are based off the gptel wiki. But I'm also continuously reading tool implementations in other languages and MCP/Integrations. The All llms will regularly fail to apply a patch, but with small changes to the doc strings, it makes a huge difference in the llms ability to use them productively:
With the initial set of tools from the wiki, no. But, they made me interested to want to have more(; Initially, I did try other ai assistants (like aider) and I used to have better agentic results with them. With the current set of tools, I have pretty good results in gptel, as well! And since it's super easy to integrate new tools (like search), gptel agentic coding often works even better than the other ai assistants. I'm still not 'satisfied' in the sense that I continue to see room for improvement and will continue to work on better tools. However, I am quite happy with the ability to do agentic coding in gptel. Here are some examples that were fully vibecoded. One was given only the GH issue (easy, because I use magit/forge and Emacs already sees the issues and gptel allows arbitrary buffers as context🙌) and was a one-shot. The other two were produced 100% by gptel, as well, but I did prompt multiple times and gptel used runcommand for tests, building docker images, finding relevant files, and iterated completely on it's own until the task was done.
A big game changer was to have filesystem tools, search and runcommand without supervision. That's only possible, because I now have an easy to use and safe sandboxing option. I also have a default system prompt which has worked well for me for a long time: https://github.com/munen/emacs.d/blob/f78523a0e60cda0dea8ea4005aa8260c35b5f7c5/custom-settings.el#L20 I only have three other system prompts that are used to generate specific kinds of text (business text, zen temple emails, meeting minutes). If you are interested, I could ping you once I have a set of tools that I'm 'fully' happy with. There's a few tweaks in my backlog that I planned to do anyway. |
Documenting a intermediary solution that I'll keep refining and testing. For a mixed context with context like: I'm getting this in
One issue is left at least: When a complete file is given, not via the buffer. |
From @munen:
From @karthink:
To add another idea to the mix, my solution has been to use the project-relative path (if possible), which is a reasonable middle ground for me. This approach does not work perfectly in all cases (ex. if the tool needs a full path, or if you have files from multiple projects in context), and perhaps the project relative path may have sensitive components. However, it is exactly what I need for my tool calls without using full paths. The version of gptel-context--insert-file-string I use is: (defun gptel-context--insert-file-string (path)
"Insert at point the contents of the file at PATH as context."
(insert (format "In file `%s`:"
(let ((project (project-current nil (file-name-directory path))))
(if project
(file-relative-name path (abbreviate-file-name (project-root project)))
(file-name-nondirectory path))))
"\n\n```\n")
(insert-file-contents path)
(goto-char (point-max))
(insert "\n```\n")) We might benefit with a |
NB: My apologies. I did not intend to close this. I made a mistake fixing a merge conflict, so I started from the current |
This commit changes the gptel context format from a simple markdown block to a structured XML format. - Uses `<context_item>` tags for each context source. - Wraps all user content in `<![CDATA[...]]>` for safety. - Adds precise `start_line` and `end_line` numbers for all content.
I have extensively tested this in the last two weeks with agentic coding sessions every day. My proposal was an improvement on the status quo, but I learned some things. I just pushed another refactor. In my the context to be used correctly by llms in combination with, it needs not just the actual file/buffer content, but it needs meta data as well as a specific prompt. Otherwise the context will be disregarded often. Here's an example output of the latest iteration:
I'll continue testing, but I'm quite confident that this is a major improvement on the status quo. |
Thank you for picking an interest in this feature! The more people who can test, the merrier👍 As for the specific proposal, this is what I would regarding a project-relative path as a default: In this thread we already have multiple scenarios in which it won't work:
I'm not against having such a flag for people who want it. But I would strongly discourage a default which is known to fail in many cases. I'd argue that the default should lead to a UX of least surprise. |
TL;DR: Even the strongest LLM currently is way bad when a file path is not as clearly specified as possible. To add one more data point about how bad LLMs are with regards to using not 100% specific file paths and working dirs, this just happened in my work project:
Here's an excerpt of what the context looked like for one of those files: <context_item>
<file_path>~/src/200ok/alephdam/modules/ravenna/src/ravenna/some_file.clj</file_path>
<file_type></file_type>
<content start_line="1" end_line="236">
<![CDATA[
discarded for this comment
]]>
</content>
</context_item> Now, behold the function call that Gemini wanted to do when trying to edit this file: "functionCall": {
"name": "apply_diff",
"args": {
"working_dir": "/home/bastian/src/200ok/alephdam/modules/ravenna/",
"file_path": "~/src/200ok/alephdam/modules/ravenna/src/ravenna/some_file.clj",
... My name is not bastian, neither is my username. The string 'bastian' is neither part of the context, the whole codebase that gptel has access to, my Emacs config or the prompt used. The shortest Levensthein distance to 'bastian' is in the that one of the files included metadata for a picture. The photographer is called "Sebastian Redacted". I don't know if it picked it up from there. In any case, I'd say it is completely made up. Since I couldn't explain it, I confronted Gemini about it. Here's what it said:
I do get the security concerns about giving away the username, and I did think that using the
|
For the time being, I'll add to the prompt that |
The model was previously making an incorrect assumption by expanding the tilde (~) character in file paths to an absolute path (e.g., /home/someuser/). This caused tool commands to fail when the inferred username did not match the actual user's environment.
@munen Let's merge the one-line change of using
I'm not sure about this prompt. Depending on the value of
Why the |
Hi @karthink
Thank you for offering to keep my contribution 🙏 I'm completely fine if you want to do the change yourself.
I see. I haven't set the flag to Tbh, I don't understand the value proposition of setting the flag to Do you have a suggestion how to proceed here?
The reason for using |
I have used this PR as is for over a month now and am quite happy with it. It's not perfect, but in my experience a big leg up to |
* gptel.el (gptel--insert-file-string): Include the full file path in the context for included files. The file name is passed through `abbreviate-file-name'. * NEWS (New features and UI changes): Mention change and reason.
I made the change to use |
Sure, let's get back to this whenever is good for you. I'm also happy to address anything that you're bringing up. Thank you for your continued efforts in gptel 🙏 |
* gptel.el (gptel--insert-file-string): Include the full file path in the context for included files. The file name is passed through `abbreviate-file-name'. * NEWS (New features and UI changes): Mention change and reason.
* gptel.el (gptel--insert-file-string): Include the full file path in the context for included files. The file name is passed through `abbreviate-file-name'. * NEWS (New features and UI changes): Mention change and reason.
Show complete file paths when adding files to context instead of just the basename. This enables LLMs to:
Longer explanation:
Problem
Currently, when adding files to the LLM context via
gptel-context
, only the filename (basename) is displayed to both the user and the LLM. This creates two significant limitations:Tool/Function calling: When LLMs need to use tools or functions that operate on files, they require the full path to correctly reference the file. Providing only the filename makes it impossible for the LLM to construct valid file operations.
Semantic context loss: File paths often contain meaningful semantic information about the codebase structure. For example:
docs/api/README.md
vsdocs/user/README.md
vsREADME.md
The directory structure provides crucial context about the file's purpose and relationship to other parts of the project.
Solution
This change modifies the context insertion functions in
gptel-context.el
to display the full file path instead of just the filename:gptel-context--insert-file-string
: Now shows full path in the context header