Add contextSummary variable to AI system

[colin-grant-work]

• Adds context to end of Coder agent's prompt
• Adds a function to allow agents to supplement the context

What it does

Fixes #14895, Closes #14945

How to test

1. Add a file to the context following the instructions in feat(ai): Enable Context Variables for Chat Requests #14787
2. Then ask Coder about something in the context. The agent should know what you're talking about.
3. Ask Coder to add something to the context. It should succeed in doing so, and the file should pop up in the UI.

Follow-ups

1. I notice that files in the context are getting labeled with a leading slash, making them appear absolute. We could address that, if we think it's confusing.
2. I also notice that if you drag+drop a file from the explorer tree onto the AI input widget, the text input gets populated with the file information:

Breaking changes

• This PR introduces breaking changes and requires careful review. If yes, the breaking changes section in the changelog has been updated.

Attribution

Review checklist

• As an author, I have thoroughly tested my changes and carefully followed the review guidelines

Reminder for reviewers

• As a reviewer, I agree to behave in accordance with the review guidelines

[JonasHelming]

Great, thank you for this integration!! The whole context story really significantly improves the work with Coder.

I did not look at the code yet, but tested a few things. My observations so far:

Case 1: Context added in chat

This worked before and the additional context summary does nothing bad (as expected).

Case 2: Context in the chat + some files only in the context

When I add additional files without any further information, and it was not related to the task, the LLM did not even look at it. When I provided an additional file that was actually relevant for the task, the LLM looked at it, it probably guessed that from the file name and the context. Looks definitely good enough to merge this to me, but we have to evaluate in practise.

Case 3: Context only in the context, not in the chat

It looked at the file(s) most of the time. Only if the prompt does not imply any implementation task, it did not. Example:
"Do not display a message to the user anymore about the tools" with context mcp-command-contribution.ts => It said: OK I won't :-D

So from case 2 and 3 we identify a Potential issue 1 that the LLM does not look at some files only in the context, but the user expects it to. I found only one prompt though, where this was really an issue. We should evaluate this in practice. If it is a problem, we can augment the prompt in iterations (during internal test) , e.g.:

• "Look at them if you need them for your task"
• "Always look at them to understand your task"

So issue 1 is not a blocker for this PR.

Case 4: Agent builds the context by itself

Issue 2: If a file is added to the context multiple times, it appears there multiple times (@planger I believe this is an issue we should solve in the context layer, WDYT?)

Issue 3 With my prompts, the LLM rather rarely "remembered" a file by itself.

I tried two options how to fix this:

Option A: Add the context add function to a sub bullet in the Context Retrieval section "If you find a file interesting for the task, remember it" => The issue here was that the LLM tended to add irrelevant files, but this needs to be evaluated.

Option B: Add the context function to a sub bullet in the apply change area "- Remember changed file: Use using~{context_addFile} to remember all files you change and files that were needed to create the changes." => Seemed to have better results than A as it does that in a phase, were it implements.

Option C: We add all modified files to the context in the modify functions ourselves (did not try). This very likely makes sense anyways.

Option A and B can be tried "on the go" as they are prompt modifications. Option C we have to implement.

However, for all options, it might help to let the LLM specify a description for what is in the file or that is has been changed.

My current option would be to go for Option C as a default.
Reasoning:

• Our current agents do not tend to look at "other relevant files" by itself, e.g. for interface declarations. So the user has to typically provide these themselves and the LLM might add irrelevant things.
• We have the context add function in place and can evaluate it more in practice with a prompt variant.

All described issues IMHO do not block this PR, we can handle them in follow-ups

Thanks again for this great addition!

[planger]

Issue 2: If a file is added to the context multiple times, it appears there multiple times (@planger I believe this is an issue we should solve in the context layer, WDYT?)

Yes, this can be considered a bug. We shouldn't add the same element twice. I'll put it on my list.

Edit: #14979

[JonasHelming]

A pretty 'satisfying' user prompt for me to test this was the following, works well with 4o, o3-mini and Sonnet 3.5. it is actually the follow-up of this PR, so the PR follow-ups itself :-)

"
@coder Add a new variable to #file:packages/ai-chat/src/common/context-summary-variable.ts that fully resolves the elements in the context including their content. Look at #file:packages/ai-ide-agents/src/browser/context-functions.ts on how to resolve the context. Now add this variable to the prompt of the universal agent (which is define in #file:packages/ai-ide-agents/src/common/universal-chat-agent.ts). similar to how it is used in the prompt of the coder agent.
"
(packages/ai-ide-agents/src/common/coder-replace-prompt-template.ts was added manually to the context only)

Note that you should type the file variables out, as they are not added to the context when pasting (the prompt of course still works, but for the test this is important)

I literately adapted the system prompt to the one below. This worked pretty nicely, but it did not add files to the context by itself, so I still believe we should go with the following prompt:

"
You are an AI assistant integrated into Theia IDE, designed to assist software developers with code tasks. You can interact with the code base and suggest changes.

Context Retrieval

Use the following functions to interact with the workspace files if you require context:

• ~{getWorkspaceDirectoryStructure}: Returns the complete directory structure.
• ~{getWorkspaceFileList}: Lists files and directories in a specific directory.
• ~{getFileContent}: Retrieves the content of a specific file.
• ~{context_addFile}: Remember file locations that are relevant for completing your tasks. Only add files that are really relevant to look at later.

Propose Code Changes

To propose code changes or any file changes to the user, never print code or new file content in your response.

Instead, for each file you want to propose changes for:

• Always Retrieve Current Content: Use getFileContent to get the latest content of the target file.
• Remember changed file: Use ~{context_addFile} to remember all files you change and files that were needed to create the changes.
• Change Content: Use ~{changeSet_writeChangeToFile} or ~{changeSet_replaceContentInFile} to suggest file changes to the user. Only select and call one function per file.

Additional Context

The following files have been provided for additional context. Some of them may also be referred to above. Always look at the relevant files to understand your task using getFileContent
{{contextSummary}}
"

[JonasHelming]

Thought about it a bit more and I believe instead of option C, we should add a resolved version of the changeset (paths and state) to the prompt with another variable. This is now very easy to do, as we have the infrastructure in place. reasoning: the previously changed files are always interesting. I can add this in a follow-up PR.

Then we can go for option A with this PR and optimized from there.

@planger WDYT?

[planger]

Thank you very much! Code looks great to me and seems to work fine.

My main concern is how we make the context summary as interpretable as possible for the LLM and give the context variable provider more control over it (see my inline comment).

Thought about it a bit more and I believe instead of option C, we should add a resolved version of the changeset (paths and state) to the prompt with another variable. This is now very easy to do, as we have the infrastructure in place. reasoning: the previously changed files are always interesting. I can add this in a follow-up PR.

Then we can go for option A with this PR and optimized from there.

I would agree. I think for now keeping those separated is simpler and more flexible. If we observe that it confuses the LLMs (which might be the case), we can reconsider. But we'll need more evaluation to confirm this even is a problem we would need to solve. :-)

[planger]

As there might be other types of variables, we should probably also mention the variable name to facilitate the interpretation of the args for the LLM, and also (or even instead of the arg) show the variable.value which is intended to be the identifier of the context variable element (e.g. the workspace relative path). The variable.value is also what's added to the user request instead of the variable, if the user uses this variable in the user request. So it is already a value that is intended to be understood by the LLM. (Edit: while the args is more like a technical id, imho)

I think it might even be worth introducing another optional property in the ResolvedAIContextVariable, something like contextSummary, with which the context variable provider can specify an LLM-optimized summary of this context element which can be printed in the context summary to the LLM.

For instance, we may have a variable that represents symbols in a source code file, e.g. #symbol:<file-uri>@<symbol-name>. Here it'd be helpful for the LLM to (a) know that it is a Symbol and (b) we may want to add additional infos, like it is a property of a specific Typescript class.

Based on the contextSummary the CONTEXT_SUMMARY_VARIABLE could output the context variables like this:

• variable.name: variable.description? // for each variable-type for which we have a variable request
  • variable.contextSummary ?? variable.value // for each variable request of the variable type
  • ...
• ...

[colin-grant-work]

@planger, thanks for the suggestion. While I like the idea, and, I've basically implemented it for the new contextDetails variable for use in agents other than Coder, it has the downside that the value field isn't available unless the variable has been resolved, and I don't think we have access to resolved variables at this point - after all, we're busy resolving one ourselves. Or is there somewhere where resolved values are cached (and would it be safe to use that cache)?

We could use the static description value associated with the variable itself, something like the file variable's 'Resolves the contents of a file'. That's not really directed at the LLM, but we could add a field that would be.

[planger]

Please see my suggestion on the variable dependencies.

Regarding the description, I totally agree. This was a misconception on my end. The description is not targeted at the LLM and therefore is probably in most cases not a great fit.

[JonasHelming]

Let's go with this prompt then: #14971 (comment)

[JonasHelming]

for #14945 we should directly add a second function to this PR, something like the follewing (not compiling semi-pseudo code)

export const CONTEXT_RESOLVE_VARIABLE: AIVariable = {
    id: 'contextResolve',
    description: nls.localize('theia/ai/core/contextResolve/description', 'Resolves the full content of context elements'),
    name: 'contextResolve',
};

@injectable()
export class ContextResolvedVariableContribution implements AIVariableContribution, AIVariableResolver {
    registerVariables(service: AIVariableService): void {
        service.registerResolver(CONTEXT_RESOLVE_VARIABLE, this);
    }

    canResolve(request: AIVariableResolutionRequest, context: AIVariableContext): MaybePromise<number> {
        if (request.variable.name === CONTEXT_RESOLVE_VARIABLE.name) {
            return 50;
        }
        return 0;
    }

    async resolve(request: AIVariableResolutionRequest, context: AIVariableContext): Promise<ResolvedAIVariable | undefined> {
        if (!ChatSessionContext.is(context) || request.variable.name !== CONTEXT_RESOLVE_VARIABLE.name) { return undefined; }

        return {
            variable: CONTEXT_RESOLVE_VARIABLE,
            value: (() => {
                context.variables.map((contextElement: any) => {
                    const variable = context.session.context.getVariables().find(v => v.id === contextElement);
                    return variable ? `Content-ID: ${variable.arg}\n + Content: ${variable.contextValue}` : 'Variable not found.'
                }
                
            })()
        }
    };
}

[JonasHelming]

@colin-grant-work Please test whether declining a changeste element and clearing the changeset works on this branch, I believe it did not when I tested, it works on master. But I am not sure

[colin-grant-work]

@planger, @JonasHelming, I've pushed a new version of the code that addresses some of the comments above, but there is likely still some work to do re: formatting things as intelligibly as possible.

• I have added two new variables, one for change sets (added to Coder's system prompt), and one for context details, added to the universal agent's prompt.
• Re: context details, @planger proposed adding various details of resolved variables to the contextSummary variable. Since that variable is intended for use with Coder, which can resolve some details on its own, that seems perhaps overkill, but I have attempted to resolve the variables in context for the contextDetails variable. Let me know what you think about that approach, but also let me know if there's some way to access resolved variables while resolving other variables; then we could use them as we see fit in both cases, and not resolve them ourselves, as I've done for contextDetails.
• Since there's a lot of data for contextDetails, I've just JSON stringified what's available - maybe there's a better way? Maybe I should do that for the change set, as well?
• I've also fixed the bug re: display of change set deletions caused by consolidation of the delete and remove events.

[planger]

Thank you very much for this iteration! I think this goes into the right direction.

I didn't consider the variable dependency issue before (sorry for that), but I feel this is a valid general use case. So I suggest to explicitly support dependent variables.

We need to ensure that we

• Don't inject the variable service into variable resolvers (cyclic DI)
• Aim at taking care of cyclic variable dependencies
• Avoid duplicate resolution

We might be able to address those constraints by handing in a resolveDependency callback to resolvers and enhance the variable resolution method.

Extend the variable resolver

export interface AIVariableResolverWithVariableDependencies extends AIVariableResolver {
    resolve(
        request: AIVariableResolutionRequest,
        context: AIVariableContext,
        resolveDependency: (req: AIVariableResolutionRequest) => Promise<ResolvedAIVariable | undefined>
    ): Promise<ResolvedAIVariable | undefined>;
}

function isResolverWithDependencies(resolver: AIVariableResolver | undefined): resolver is AIVariableResolverWithVariableDependencies {
    return resolver !== undefined && resolver.resolve.length >= 3;
}

Enhance the resolveVariable method of the variable service

interface CacheEntry {
    promise: Promise<ResolvedAIVariable | undefined>;
    inProgress: boolean;
}

export class DefaultAIVariableService implements AIVariableService {
    // ... existing properties and methods

    async resolveVariable(
        request: AIVariableArg,
        context: AIVariableContext,
        cache: Map<string, CacheEntry> = new Map()
    ): Promise<ResolvedAIVariable | undefined> {
        const variableName = typeof request === 'string'
            ? request
            : typeof request.variable === 'string'
                ? request.variable
                : request.variable.name;

        if (cache.has(variableName)) {
            const entry = cache.get(variableName)!;
            if (entry.inProgress) {
                this.logger.warn(`Cycle detected for variable: ${variableName}. Skipping resolution.`);
                return undefined;
            }
            return entry.promise;
        }

        const entry: CacheEntry = { promise: Promise.resolve(undefined), inProgress: true };
        cache.set(variableName, entry);

        const promise = (async () => {
            const variable = this.getVariable(variableName);
            if (!variable) {
                return undefined;
            }
            const arg = typeof request === 'string' ? undefined : request.arg;
            const resolver = await this.getResolver(variableName, arg, context);
            let resolved: ResolvedAIVariable | undefined;
            if (isResolverWithDependencies(resolver)) {
                resolved = await resolver.resolve(
                    { variable, arg },
                    context,
                    async (depRequest: AIVariableResolutionRequest) =>
                        this.resolveVariable(depRequest, context, cache)
                );
            } else {
                resolved = await resolver?.resolve({ variable, arg }, context);
            }
            return resolved ? { ...resolved, arg } : undefined;
        })();

        entry.promise = promise;
        promise.finally(() => {
            entry.inProgress = false;
        });

        return promise;
    }
}

Summary

• AIVariableResolverWithVariableDependencies lets a resolver access dependencies through a callback during its resolution.
• The updated resolveVariable method uses a promise-based cache with an inProgress flag for cycle detection. This way, if a variable is already being resolved, it’s skipped—avoiding cycles.
• With this approach, resolved variables are accessible during the resolution of dependent ones. This could be leveraged for both contextSummary (letting Coder resolve details) and contextDetails, reducing redundant resolution.

What do you think about this approach?

Maybe I'm over-complicating this, but I feel like letting variables access other variables might be a useful thing in general, and if it is, we should explicitly support it.

If you or others disagree that we should support that, I'm happily follow your lead and think your approach is fine, given it is rather local and puts the responsibility of using this approach carefully into the hands of a specific variable resolver.

[planger]

I think we should name this ChangeSetSummaryVariableContribution right?

[planger]

Currently this variable is targeted at files, even though the context may contain arbitrary context variables, like symbols or even domain-specific things.

If we want to keep it that way, we should also reflect that in the naming. It is already reflected in the description.

In general, I tend to believe we can generalize that, because the only file specific thing is currently the description and the word "file" in line 60. Both can be avoided by using the contextSummary field or the variable name.

[planger]

You are probably right to remove this.

[planger]

I still think it might be worth introducing a dedicated summary field here, but I'm certainly not insisting on it if you feel it isn't necessary or if we should rather come back to it at a later point, if the requirement manifests.

[colin-grant-work]

@planger, following our discussion, my latest push mostly cleans up the existing implementation.

• I've corrected the naming of and bound the ContextDetails, ContextSummary, and ChangeSet variable contributions.
• I've removed the context summary variable's focus on files and exposed the context value resolution function to the both the Architect and Coder agents so that they can context items generically.
• I've exposed the ContextDetails to the universal agent
• I've updated the various places we pass context into variable resolvers to consistently use the ChatSessionContext interface where available.

There is one outstanding question from #14945, namely whether we want to provide more details than the file names and status and provide e.g. the full text of a given suggestion.

Re: follow ups, I have not implemented your suggestions re: explicit support for dependent variables. In the event, it wasn't necessary because we already pre-resolve the context, with which the variables in this PR are concerned. In my view, we shouldn't eagerly resolve the context, because we don't know that it will actually be used, but we should implement some kind of variable caching (perhaps on the request level?) and better support for variable dependencies, since if we don't eagerly resolve context variables, then the context summary will definitely fall into that category.

You'll also see that I've changed the passing of the context a bit: rather than having it passed in as an argument to the chat service, I've located it on the ChatModel. That makes it easier to pass around as part of variable resolution - e.g. when handling system messages - but if you believe that it's important that the context be separated from the model, I'm open to discussion.

[JonasHelming]

Note: we just decided in a direct discussion to split the generic context summary variable provided by the framework and the one used by Coder and Architect. the reasoning is that we want to make it maximum simple for Coder and Architect and as the can already access files, they just need the path. Furthermore, as they are part of the IDE, we have under control, which additional future context element types they have to deal with and can handle this explicitly. Having the generic variable in the framework provides significant value for adopters of Theia AI, as they can directly use this as a good default. Specifically if their agents use the context as a primary source for information, which is a common case I guess.

@planger

[JonasHelming]

I tested Coder, Architect and Universal and did not find any case, where this does not work well so far. I added some minor suggestions.
I will let Philip apporve this, if the framework part is fine.

Great work, thank you

[planger]

Thank you very much @colin-grant-work for this excellent work and for pushing through all of the discussions we had on this PR!

This looks great and works very well. I didn't test in detail as @JonasHelming already did, but what I tested seemed pretty good.

I don't have any comment that should block merging this change, just a few details that may well be first evaluated in practice or that can be done in a follow-up. Besides my inline comments, I'd suggest to rename and move:

• ChangeSetVariableContribution
• ContextSummaryVariableContribution

Both are somewhat file-specific at the moment, which is fine as discussed---we can generalize them later, but then we should probably move them to ai-ide and rename them. This would then also be in line with e.g. the ContextFilesVariableContribution.

Even that however, can be done in a follow-up after the pre-release as it doesn't impact the behavior at all. So I'm fine to merge this and we'll create follow-ups.

[JonasHelming]

Tested again with the minor updates, no issues found