Add code for triggering completion requests to the provider [devtools/devtools-frontend : main]
Ergün Erdoğmuş (Gerrit)
Ergün Erdoğmuş voted and added 8 comments![]( "Open in Gerrit")
Votes added by Ergün Erdoğmuş
| Code-Review | +1 |
8 comments
{aidaClient: this.#aidaClient}, this.#aiCodeCompletionConfig.panel, undefined,Let's add a `TODO` to remove the `panel` from here so that we don't forget removing it as it won't be used after the migration.
return accepted;(nit): Consider refactoring the `Tab` handler's `run` method to use early returns (guard clauses). This can improve readability by reducing nesting, as advocated in tott/733.
The current structure has a couple of levels of nesting (preconditions check -> `accepted` check). With early returns, we can flatten this.
The benefit of guard clauses here isn't just less indentation, but also a more linear cognitive flow. With the current nesting, one might first read *what* the block does, and then have to work outwards to understand *when* those actions are actually performed.
Guard clauses invert this. You check preconditions sequentially:
1. Are the basic components available? If not, return `false`.
2. Was the suggestion accepted? If not, return `false`.
If all guards pass, the reader knows the subsequent unnested code block is executed. This feels more like a step-by-step process, reducing the mental overhead of tracking nested contexts. You don't need to read the inner logic if the guard conditions aren't met.
Here's how it could look:
```typescript
run: (): boolean => {
// Guard clause for preconditions
if (!this.#aiCodeCompletion || !this.#editor || !hasActiveAiSuggestion(this.#editor.state)) {
return false;
}
const {accepted, suggestion} = acceptAiAutoCompleteSuggestion(this.#editor.editor);// Guard clause for non-acceptance
if (!accepted) {
return false;
}
// Main logic - now unnested
if (suggestion?.rpcGlobalId) {
this.#aiCodeCompletion?.registerUserAcceptance(suggestion.rpcGlobalId, suggestion.sampleId);
}
this.#aiCodeCompletionConfig?.onSuggestionAccepted();
return true; // It's now obvious that accepted must be true to reach this point.
}
```
This structure makes the happy path very clear and unindented.
if (!update.docChanged || update.state.doc === update.startState.doc) {Do we need the `update.state.doc === update.startState.doc` check here? Can't we rely on `update.docChanged`? I think we can also combine the guard clause here to check for `!this.#editor` and `!this.#aiCodeCompletion` as well.
const query = doc.toString();(I know that this is getting copied from the previous logic) though, while we're here, we can improve it :)
1. We don't need to stringify the whole document, let's use `sliceString` to grab the prefix & suffix. We can do something like:
```
const prefixStart = Math.max(0, cursor - AI_CODE_COMPLETION_CHARACTER_LIMIT);
let prefix = doc.sliceString(prefixStart, cursor);
```
2. It's a low chance but to make this call idempotent, let's use the state from the `update`. We can do: `const {state: {doc, selection} = update`.
3. We don't add `completionContextPrefix` to the `prefix` character trimming. Is that intended?
4. We can name `AI_CODE_COMPLETION_CHARACTER_LIMIT` to `MAX_PREFIX_SUFFIX_LENGTH` since it's not the limit of the characters we're sending to it but rather it's the limit we selected for the `prefix` and `suffix` parts of the request.
AiCodeCompletion.AiCodeCompletion.AIDA_REQUEST_DEBOUNCE_TIMEOUT_MS);Let's use the `AIDA_REAQUEST_DEBOUNCE_TIMEOUT_MS` defined in this module. We'll remove the ones defined in the `AiCodeCompletion` anyways after we migrate both Sources & Console panels to use `AiCodeCompletionProvider`.
additionalFiles?: Host.AidaClient.AdditionalFile[]): Promise<void> {Since it'd be an error to call `#requestAidaSuggestion` without `#aiCodeCompletion`, WDYT of throwing here or logging an error and returning early if the `#aiCodeCompletion` is not initialized?
this.#aiCodeCompletionConfig?.onResponseReceived([]);(nit): Let's add a TODO here to move exposing citations from `onSuggestionAccepted` since we only need to show them when the suggestion is accepted.
Related details
- Samiya Caur
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |