Skip to content

feat/autofix: get explanations from agent fix response, fall back to AI Explain - #150

Open
BBerabi wants to merge 2 commits into
mainfrom
feat/autofix/improved-explanations
Open

feat/autofix: get explanations from agent fix response, fall back to AI Explain#150
BBerabi wants to merge 2 commits into
mainfrom
feat/autofix/improved-explanations

Conversation

@BBerabi

@BBerabi BBerabi commented May 28, 2026

Copy link
Copy Markdown

Description

AI Explain is being deprecated. Newer Agent Fix / Autofix responses already include an explanation per suggestion, so we no longer need a separate call to the (deprecated) AI Explain service in that case.

This PR:

  • Propagates the explanation field from the sast-autofix-api response through to AutofixUnifiedDiffSuggestion.Explanation.
  • Adds AutofixOptions.ExplainEndpoint. When set, GetAutofixDiffs falls back to the deprecated AI Explain service, but only for suggestions whose response did not already include an explanation. Suggestions that already have one are left untouched, and the AI Explain call is skipped entirely if none are missing or no ExplainEndpoint is configured.

This keeps the decision of "do we still need AI Explain" in one place (code-client-go) rather than duplicated across every consumer (snyk-ls, remy-cli-extension, native-pr-experience), so it supports both flows during rollout without a coordinated multi-repo change.

Checklist

  • Tests added and all succeed
  • Linted
  • README.md updated, if user-facing

🚨After having merged, please update the snyk-ls and CLI go.mod to pull in latest client.

Consumers that want the fallback need to pass ExplainEndpoint in AutofixOptions and can drop their own "call AI Explain unconditionally" logic (e.g. AiFixHandler.EnrichWithExplain in snyk-ls).

@BBerabi
BBerabi requested a review from a team as a code owner May 28, 2026 17:56
@snyk-io

snyk-io Bot commented May 28, 2026

Copy link
Copy Markdown

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues
Code Security 0 0 0 0 0 issues
Secrets 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@snyk-io

snyk-io Bot commented May 28, 2026

Copy link
Copy Markdown

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues
Code Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@CLAassistant

CLAassistant commented May 28, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@snyk-pr-review-bot

This comment has been minimized.

@BBerabi
BBerabi marked this pull request as draft June 4, 2026 08:55
@BBerabi
BBerabi marked this pull request as ready for review June 11, 2026 12:45
@snyk-pr-review-bot

This comment has been minimized.

… explanation

AI Explain is being deprecated in favor of explanations returned directly by
Autofix/Agent Fix. GetAutofixDiffs now falls back to the deprecated AI Explain
service only for suggestions missing an explanation, so both consumers on the
old backend and the new one are supported during rollout without duplicating
this decision in every caller.
@BBerabi BBerabi changed the title feat/autofix: get explanations directly from agent fix response feat/autofix: get explanations from agent fix response, fall back to AI Explain Jul 13, 2026
@snyk-pr-review-bot

Copy link
Copy Markdown

PR Reviewer Guide 🔍

🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Ordering Mismatch 🟠 [major]

enrichWithExplain maps fallback explanations to suggestions using positional indexing derived from getOrderedResponse. However, getOrderedResponse (line 173) performs a lexicographical sort on map keys. If the API returns 10 or more explanations (e.g., 'explanation1' through 'explanation10'), the sort order will be 'explanation1', 'explanation10', 'explanation2', etc. This will cause explanations to be incorrectly assigned to the wrong fix suggestions for any index greater than 1.

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]
Inefficient Concatenation 🟡 [minor]

The concatDiffs function uses repeated string concatenation (diff += v) inside a loop. In Go, strings are immutable, so this pattern results in O(n^2) time complexity and unnecessary memory allocations as new strings are created in every iteration. For suggestions involving multiple files or large diffs, this should use strings.Builder.

func concatDiffs(suggestion AutofixUnifiedDiffSuggestion) string {
	diff := ""
	for _, v := range suggestion.UnifiedDiffsPerFile {
		diff += v
	}
	return diff
📚 Repository Context Analyzed

This review considered 12 relevant code sections from 11 files (average relevance: 0.93)

🤖 Repository instructions applied (from AGENTS.md)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants