Ghost context provider #3506
Ghost context provider #3506
Conversation
…nd recentlyEdited
|
markijbema
left a comment
markijbema
left a comment
There was a problem hiding this comment.
I'd expect there to be code to register/deregister the ContextRetrievalService. So either it does it itself, but in that case there is some missing teardown code, or it doesnt, in which case it will always give an empty result.
Either way, even with that this does look way simpler than expected! Very cool
…g/kilocode into beatlevic/ghost-context-provider
src/services/ghost/classic-auto-complete/GhostContextProvider.ts
Outdated
Show resolved
Hide resolved
src/services/ghost/classic-auto-complete/GhostContextProvider.ts
Outdated
Show resolved
Hide resolved
src/services/ghost/classic-auto-complete/GhostContextProvider.ts
Outdated
Show resolved
Hide resolved
| filepath: filepathUri, | ||
| } | ||
|
|
||
| const helper = await HelperVars.create(helperInput as any, DEFAULT_AUTOCOMPLETE_OPTS, "codestral", this.ide) |
There was a problem hiding this comment.
what is the codestral doing here? Is it doing something codestral specific?
There was a problem hiding this comment.
I ended up pushing up a fix to plumb through out model. Verified that the continue helper code ought to work with our model descriptors
There was a problem hiding this comment.
I don't follow this, alas - why is it safe+desirable to do something codestral specific here?
There was a problem hiding this comment.
This is related to how the HelperVar needs to be initialized, and it does use the model to prune the prefix in certain cases. Let me at the very least pass our actual model name in here.
| // Convert all snippet filepaths to URIs | ||
| const snippetsWithUris = filteredSnippets.map((snippet: any) => ({ | ||
| ...snippet, | ||
| filepath: snippet.filepath?.startsWith("file://") |
There was a problem hiding this comment.
again, why do we have this in two different formats; is this a real thing or just llm slop?
There was a problem hiding this comment.
This was actually needed because the continuedev utilities expect URIs
There was a problem hiding this comment.
If continuedev expects URIS, is there a reason we don't always use uris? why check here?
There was a problem hiding this comment.
Good question, and something we can improve on. I'm not even sure where the non-URIs come from.
src/services/ghost/classic-auto-complete/GhostContextProvider.ts
Outdated
Show resolved
Hide resolved
EamonNerbonne
left a comment
EamonNerbonne
left a comment
There was a problem hiding this comment.
A few questions I don't quite understand the answers to.
However most important is that this is going in the right direction, and also that it doesn't make things worse in the interim - the fact that you ran the evals is evidence of that.
Should some of the open questions be effectively techdebt that's hard to solve here and now but worthwhile accepting, I think we should make an issue tracking these choices so we can revisit them after autocomplete is live (and then see how much can or should be fixed once the pieces fit into the puzzle better).
…n llm test benchmark.
Added ghost context provider that retrieves all relevant context in a comment wrapped style.
Adds the comment wrapped code to the QUERY prompt.