diff --git a/llm/api_client.go b/llm/api_client.go index ddbba2c..68c5211 100644 --- a/llm/api_client.go +++ b/llm/api_client.go @@ -76,7 +76,7 @@ func (d *DeepCodeLLMBindingImpl) submitRequest(ctx context.Context, url *url.URL return nil, err } - req, err := http.NewRequestWithContext(ctx, http.MethodPost, url.String(), bodyBuffer) + req, err := http.NewRequestWithContext(span.Context(), http.MethodPost, url.String(), bodyBuffer) if err != nil { logger.Err(err).Str("requestBody", string(requestBody)).Msg("error creating request") return nil, err diff --git a/llm/binding.go b/llm/binding.go index cfdc9f4..b36ba6d 100644 --- a/llm/binding.go +++ b/llm/binding.go @@ -88,7 +88,72 @@ func (d *DeepCodeLLMBindingImpl) GetAutofixDiffs(ctx context.Context, _ string, if err != nil { return nil, status, err } - return autofixResponse.toUnifiedDiffSuggestions(d.logger, options.BaseDir, options.FilePath), status, err + + unifiedDiffSuggestions = autofixResponse.toUnifiedDiffSuggestions(d.logger, options.BaseDir, options.FilePath) + d.enrichWithExplain(span.Context(), options, unifiedDiffSuggestions) + + return unifiedDiffSuggestions, status, err +} + +// enrichWithExplain fills in the Explanation for suggestions whose Autofix response did not +// already include one, falling back to the deprecated AI Explain service. Suggestions that +// already carry an explanation are left untouched, and the call is skipped entirely once none +// are missing or no ExplainEndpoint was configured. +func (d *DeepCodeLLMBindingImpl) enrichWithExplain(ctx context.Context, options AutofixOptions, suggestions []AutofixUnifiedDiffSuggestion) { + method := "code.EnrichWithExplain" + logger := d.logger.With().Str("method", method).Logger() + + missingIndices := make([]int, 0, len(suggestions)) + for i := range suggestions { + if suggestions[i].Explanation == "" { + missingIndices = append(missingIndices, i) + } + } + if len(missingIndices) == 0 { + return + } + + if options.ExplainEndpoint == nil { + logger.Debug().Msg("No ExplainEndpoint configured, skipping AI Explain fallback") + return + } + + span := d.instrumentor.StartSpan(ctx, method) + defer d.instrumentor.Finish(span) + + diffs := make([]string, 0, len(missingIndices)) + for _, idx := range missingIndices { + diffs = append(diffs, concatDiffs(suggestions[idx])) + } + + response, err := d.runExplain(span.Context(), ExplainOptions{ + RuleKey: options.RuleID, + Diffs: diffs, + Endpoint: options.ExplainEndpoint, + }) + if err != nil { + logger.Err(err).Msg("Failed to obtain fallback explanations from AI Explain") + return + } + + explanations := getOrderedResponse(response) + for i, idx := range missingIndices { + if i >= len(explanations) { + logger.Debug().Msgf("Failed to get fallback explanation for suggestion index %v", idx) + break + } + suggestions[idx].Explanation = explanations[i] + } +} + +// concatDiffs concatenates the diffs of a suggestion across all its files, as the (deprecated) +// AI Explain service expects a single diff string per suggestion. +func concatDiffs(suggestion AutofixUnifiedDiffSuggestion) string { + diff := "" + for _, v := range suggestion.UnifiedDiffsPerFile { + diff += v + } + return diff } func (d *DeepCodeLLMBindingImpl) ExplainWithOptions(ctx context.Context, options ExplainOptions) (ExplainResult, error) { diff --git a/llm/binding_test.go b/llm/binding_test.go index 88b61a3..8b592e2 100644 --- a/llm/binding_test.go +++ b/llm/binding_test.go @@ -65,6 +65,67 @@ func TestExplainWithOptions(t *testing.T) { }) } +func TestEnrichWithExplain(t *testing.T) { + t.Run("skips when no suggestions are missing an explanation", func(t *testing.T) { + d, mockHTTPClient := getHTTPMockedBinding(t) + mockHTTPClient.EXPECT().Do(gomock.Any()).Times(0) + + suggestions := []AutofixUnifiedDiffSuggestion{ + {FixId: "fix-1", Explanation: "explanation 1", UnifiedDiffsPerFile: map[string]string{"a.go": "diff1"}}, + {FixId: "fix-2", Explanation: "explanation 2", UnifiedDiffsPerFile: map[string]string{"b.go": "diff2"}}, + } + endpoint := &url.URL{Scheme: "http", Host: "test.com"} + + d.enrichWithExplain(t.Context(), AutofixOptions{ExplainEndpoint: endpoint}, suggestions) + + assert.Equal(t, "explanation 1", suggestions[0].Explanation) + assert.Equal(t, "explanation 2", suggestions[1].Explanation) + }) + + t.Run("skips when no ExplainEndpoint is configured", func(t *testing.T) { + d, mockHTTPClient := getHTTPMockedBinding(t) + mockHTTPClient.EXPECT().Do(gomock.Any()).Times(0) + + suggestions := []AutofixUnifiedDiffSuggestion{ + {FixId: "fix-1", UnifiedDiffsPerFile: map[string]string{"a.go": "diff1"}}, + } + + d.enrichWithExplain(t.Context(), AutofixOptions{}, suggestions) + + assert.Equal(t, "", suggestions[0].Explanation) + }) + + t.Run("fills in only the missing explanations", func(t *testing.T) { + d, mockHTTPClient := getHTTPMockedBinding(t) + + explainResponseJSON := explainResponse{ + Status: completeStatus, + Explanation: map[string]string{ + "explanation1": "fallback explanation for fix-2", + }, + } + expectedResponseBody, err := json.Marshal(explainResponseJSON) + assert.NoError(t, err) + mockResponse := http2.Response{ + Status: "200 Ok", + StatusCode: 200, + Body: io.NopCloser(strings.NewReader(string(expectedResponseBody))), + } + mockHTTPClient.EXPECT().Do(gomock.Any()).Return(&mockResponse, nil).Times(1) + + suggestions := []AutofixUnifiedDiffSuggestion{ + {FixId: "fix-1", Explanation: "explanation from response", UnifiedDiffsPerFile: map[string]string{"a.go": "diff1"}}, + {FixId: "fix-2", UnifiedDiffsPerFile: map[string]string{"b.go": "diff2"}}, + } + endpoint := &url.URL{Scheme: "http", Host: "test.com"} + + d.enrichWithExplain(t.Context(), AutofixOptions{RuleID: "rule-key", ExplainEndpoint: endpoint}, suggestions) + + assert.Equal(t, "explanation from response", suggestions[0].Explanation) + assert.Equal(t, "fallback explanation for fix-2", suggestions[1].Explanation) + }) +} + func getHTTPMockedBinding(t *testing.T) (*DeepCodeLLMBindingImpl, *mocks.MockHTTPClient) { t.Helper() ctrl := gomock.NewController(t) diff --git a/llm/convert.go b/llm/convert.go index d229996..262aaab 100644 --- a/llm/convert.go +++ b/llm/convert.go @@ -24,6 +24,7 @@ func (s *AutofixResponse) toUnifiedDiffSuggestions(logger *zerolog.Logger, baseD d := AutofixUnifiedDiffSuggestion{ FixId: suggestion.Id, UnifiedDiffsPerFile: map[string]string{}, + Explanation: suggestion.Explanation, } d.UnifiedDiffsPerFile[decodedPath] = unifiedDiff diff --git a/llm/types.go b/llm/types.go index d7cf6f2..76e913f 100644 --- a/llm/types.go +++ b/llm/types.go @@ -72,8 +72,9 @@ type AutofixResponse struct { AutofixSuggestions []autofixResponseSingleFix `json:"fixes"` } type autofixResponseSingleFix struct { - Id string `json:"id"` - Value string `json:"value"` + Id string `json:"id"` + Value string `json:"value"` + Explanation string `json:"explanation"` } // AutofixUnifiedDiffSuggestion represents the diff between the original and the fixed source code. @@ -136,6 +137,12 @@ type AutofixOptions struct { Host string CodeRequestContext CodeRequestContext IdeExtensionDetails AutofixIdeExtensionDetails + + // ExplainEndpoint is the (deprecated) AI Explain endpoint. It is only used as a fallback to + // obtain explanations for autofix suggestions whose response did not already include one, e.g. + // when served by an older Autofix backend. If nil, no fallback call is made and suggestions + // without an explanation are returned as-is. + ExplainEndpoint *url.URL } type AutofixFeedbackOptions struct {