RFC 969 API boilerplate#63789
Conversation
| """ | ||
| Optional: Name of the file associated with this context item. | ||
| """ | ||
| fileName: String |
There was a problem hiding this comment.
Should we also add ranges within the file?
There was a problem hiding this comment.
I went back and forth on this - I don't want to mirror the whole "FileBasedContextItem" structure here (and the same thing for OpenCtx / URL / ...) so this API is easy to use, so I agree we should either remove fileName or add line ranges. WDYT - should we drop fileName for now?
There was a problem hiding this comment.
Can we please leave it in and add ranges as well? It will be really helpful for constructing datasets from logs.
| GetCodyIntent(ctx context.Context, args GetIntentArgs) (IntentResolver, error) | ||
| ChatIntent(ctx context.Context, args ChatIntentArgs) (IntentResolver, error) | ||
| RankContext(ctx context.Context, args RankContextArgs) (RankContextResolver, error) | ||
| RecordContext(ctx context.Context, args RecordContextArgs) (*EmptyResponse, error) |
There was a problem hiding this comment.
Why do we want a separate call for recording context?
There was a problem hiding this comment.
Because (in our current end-to-end implementation) the final decision to include a certain item is made on the client (ex. based on token limits), so it's possible that the context passed to the LLM is different from what was passed to/return from the ranking RPC. Does that make sense?
There was a problem hiding this comment.
Oh yeah, that makes sense. I guess we'll deprecate it in the future.
| TargetContextWindowTokens *int32 | ||
| Intent *string | ||
| Command *string | ||
| InteractionID string |
There was a problem hiding this comment.
Query shouldn't be optional. TargetModel probably as well, but we can use a default one.
There was a problem hiding this comment.
Today, target model is actually configured server-side (for chat), so it felt natural not to require the client to pass it (ex. it's not required if you call the actual Completions endpoint).
Over time, I expect most of the optional fields to become required.
There was a problem hiding this comment.
As for Query - we can look it up on the server using InteractionId, do you think we should require the client to pass it again?
There was a problem hiding this comment.
Hm, why would we want extra look ups? IMO passing it again is probably preferable.
There was a problem hiding this comment.
Ok, guess it's not too hard for the client to hold on to it
This PR adds the Client-side integration of https://github.com/sourcegraph/sourcegraph/pull/63789 for VSCode: - GraphQL queries, types and client methods - ContextAPIClient class mapping VSCode objects to GraphQL - wiring of ContextAPIClient to existing code Currently, for all added backend calls: - all remote RPC are guarded by a feature flag - results of RPCs are not used by integrating call - calls are not awaited (to avoid slowing down interactions) Related to AI-128. ## Test plan - tested locally -> works with local and sourcegraph.com backend
This PR adds experimental endpoints implementing for context ranking and storage, as required by RFC 969.
Also removes the deprecated and unused
getCodyIntentquery (we havechatIntent).Test plan