Skip to content

fix: remove suggestion blocks from inline comments#33

Merged
supasympa merged 1 commit into
mainfrom
fix/suggestion-blocks
May 11, 2026
Merged

fix: remove suggestion blocks from inline comments#33
supasympa merged 1 commit into
mainfrom
fix/suggestion-blocks

Conversation

@supasympa

Copy link
Copy Markdown
Contributor

Summary

GitHub's ```suggestion block means "replace this line of code with this exact content." The LLM suggested_fix values are natural language advice (e.g., "Use parameterized queries"), not verbatim code replacements — so rendering them as suggestion blocks caused the entire comment to appear as a proposed code change in "Files Changed."

Fix

  • Remove isSingleLineSuggestion() and the ```suggestion block rendering
  • Render SuggestedFix as a blockquote: > **Suggested fix:** ...
  • Always render SuggestedFix the same way (no more single-line vs multi-line branching)
  • Bump version to 1.12.1

Testing

  • go build ./... — clean
  • go vet ./... — clean
  • go test ./... — all pass

… fix as plain markdown

GitHub suggestion blocks (suggestion) mean code replacement, but LLM
suggested_fix values are natural language advice, not verbatim code.
Rendering them as suggestion blocks caused the entire comment body to
appear as a proposed code change in the Files Changed UI.
@supasympa supasympa merged commit 3e8fe0e into main May 11, 2026
@supasympa supasympa deleted the fix/suggestion-blocks branch May 11, 2026 18:02

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

acig ACIG · WARN · risk=high · 2 finding(s)

🟡 medium ×1 · ℹ️ info ×1

1 inline comment(s) posted on the Files changed tab. 1 finding(s) could not be anchored to a diff line — see the sticky comment for the full list.

Cost: $0.0002 · 180934ms

Comment on lines 424 to 429
return diffs[path]
}

// buildInlineCommentBody renders the markdown for a group of findings on the
// same file:line. Includes a logo header, severity badge, suggestion block
// (when SuggestedFix is single-line), and an ignore hint.
func buildInlineCommentBody(findings []verdict.Finding) string {
var body strings.Builder
body.WriteString(acigMarker)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

acig ACIG · 1 issue(s)

[🟡 medium] Missing tests for GitHubReporter.Report error handling when GitHub API returns 403 · adjudicator

The new 403 error handling branch in GitHubReporter.Report (lines 424-429) is not exercised by any test. Add a test that simulates a 403 response and verifies the expected behavior.

Suggested change
return diffs[path]
}
// buildInlineCommentBody renders the markdown for a group of findings on the
// same file:line. Includes a logo header, severity badge, suggestion block
// (when SuggestedFix is single-line), and an ignore hint.
func buildInlineCommentBody(findings []verdict.Finding) string {
var body strings.Builder
body.WriteString(acigMarker)
Add a test case in internal/reporters/github_test.go that mocks a 403 Forbidden response from the GitHub API and asserts the correct error handling logic is triggered.

To suppress: add # acig-ignore: adjudicator near this line.

@github-actions

Copy link
Copy Markdown

acig ACIG · Code Review

WARN · risk=high · 2 finding(s) · $0.0002

🟡 medium ×1 · ℹ️ info ×1

Severity Critic Title File
ℹ️ info test_coverage_smell Failed to parse critic output
🟡 medium adjudicator Missing tests for GitHubReporter.Report error handling when GitHub API returns 403 internal/reporters/github.go:424
Findings outside the diff

These findings were produced by critics but couldn't be anchored to a line in this PR's diff (e.g. references to deleted files or lines outside any hunk).

  • [ℹ️ info] Failed to parse critic output (test_coverage_smell)
    Model returned invalid JSON (531 bytes). This usually means the output was truncated. Raw: {"findings":[{"severity":"medium","title":"Missing tests for GitHubReporter.Report error handling when GitHub API returns 403","detail":"The new 403 error handling branch in GitHubReporter.Report (lines 424-429) is not exercised by any test. Add a test that simulates a 403 response and verifies the ...
Validation notes
  • test_coverage_smell: JSON parse failed; raw response truncated
Verdict JSON
{"schema_version":"1","repo":"https://github.com/helloodokai/acig","sha":"df58f429d884e4d357c5e50c3d9f6515b4921e8f","base_sha":"main","risk":"high","decision":"warn","summary":"Decision: warn | Risk: high | Findings: 0 blocking, 0 high, 1 medium, 0 low, 1 info | Cost: $0.0002","findings":[{"critic":"test_coverage_smell","severity":"info","title":"Failed to parse critic output","detail":"Model returned invalid JSON (531 bytes). This usually means the output was truncated. Raw: {\"findings\":[{\"severity\":\"medium\",\"title\":\"Missing tests for GitHubReporter.Report error handling when GitHub API returns 403\",\"detail\":\"The new 403 error handling branch in GitHubReporter.Report (lines 424-429) is not exercised by any test. Add a test that simulates a 403 response and verifies the ..."},{"critic":"adjudicator","severity":"medium","title":"Missing tests for GitHubReporter.Report error handling when GitHub API returns 403","detail":"The new 403 error handling branch in GitHubReporter.Report (lines 424-429) is not exercised by any test. Add a test that simulates a 403 response and verifies the expected behavior.","file":"internal/reporters/github.go","line_start":424,"line_end":429,"suggested_fix":"Add a test case in internal/reporters/github_test.go that mocks a 403 Forbidden response from the GitHub API and asserts the correct error handling logic is triggered."}],"critic_results":[{"critic":"style_conformance","model":"gpt-oss:20b","findings":null,"cost_usd":0.00001787,"duration_ms":2521,"tokens_in":956,"tokens_out":277},{"critic":"risk_classifier","model":"gpt-oss:20b","findings":null,"cost_usd":0.0000213,"duration_ms":3180,"tokens_in":564,"tokens_out":522,"reasoning":"Modified GitHub reporter adds new network calls, creating a new I/O surface.","risk":"high"},{"critic":"test_coverage_smell","model":"gpt-oss:20b","findings":[{"critic":"test_coverage_smell","severity":"info","title":"Failed to parse critic output","detail":"Model returned invalid JSON (531 bytes). This usually means the output was truncated. Raw: {\"findings\":[{\"severity\":\"medium\",\"title\":\"Missing tests for GitHubReporter.Report error handling when GitHub API returns 403\",\"detail\":\"The new 403 error handling branch in GitHubReporter.Report (lines 424-429) is not exercised by any test. Add a test that simulates a 403 response and verifies the ..."}],"cost_usd":0.00007083,"duration_ms":10076,"tokens_in":939,"tokens_out":2048,"notes":["JSON parse failed; raw response truncated"]},{"critic":"perf_smell","model":"qwen3-coder:480b","findings":null,"cost_usd":0.00006024,"duration_ms":30029,"tokens_in":965,"tokens_out":13},{"critic":"security_smell","model":"qwen3-coder:480b","findings":null,"cost_usd":0.00007301999999999999,"duration_ms":30843,"tokens_in":1178,"tokens_out":13},{"critic":"adjudicator","model":"glm-5.1","findings":[{"critic":"adjudicator","severity":"medium","title":"Missing tests for GitHubReporter.Report error handling when GitHub API returns 403","detail":"The new 403 error handling branch in GitHubReporter.Report (lines 424-429) is not exercised by any test. Add a test that simulates a 403 response and verifies the expected behavior.","file":"internal/reporters/github.go","line_start":424,"line_end":429,"suggested_fix":"Add a test case in internal/reporters/github_test.go that mocks a 403 Forbidden response from the GitHub API and asserts the correct error handling logic is triggered."}],"cost_usd":0,"duration_ms":104285,"tokens_in":1412,"tokens_out":1235}],"total_cost_usd":0.00024325999999999997,"total_duration_ms":180934,"budget_remaining_usd":0.24975674,"generated_at":"2026-05-11T18:02:19.085471494Z"}

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.

1 participant