Sonnet Review Bot sonnet-review-bot
  • Joined on 2026-04-24
sonnet-review-bot commented on pull request rodin/review-bot#48 2026-05-08 02:20:03 +00:00
fix: retry on transient LLM response body truncation

[MAJOR] CI is failing for all six review jobs. The verdict must be REQUEST_CHANGES regardless of code quality.

sonnet-review-bot commented on pull request rodin/review-bot#48 2026-05-08 02:20:03 +00:00
fix: retry on transient LLM response body truncation

[MAJOR] isRetryableError classifies errors by substring-matching on err.Error(). This is fragile: any future error message that happens to contain 'read response' will be silently retried, and a refactor of the error message strings in doRequest will silently break the retry logic without a compile-time signal. The idiomatic Go approach is to use sentinel errors (var errBodyTruncated = errors.New(...)) or a custom error type implementing errors.Is/errors.As, then check with errors.Is(err, errBodyTruncated). This is especially important because isRetryableError is tested by constructing errors from raw strings (fmt.Errorf("%s", tt.err)), which means the tests would still pass even if the actual error-wrapping chain changed.

sonnet-review-bot commented on pull request rodin/review-bot#48 2026-05-08 02:20:03 +00:00
fix: retry on transient LLM response body truncation

[NIT] The loop bound attempt < 2 with attempt starting at 0 is idiomatic but slightly harder to read than maxRetries = 2 with a named constant, especially now that the same retry count (2) appears in both Complete() in llm/client.go and the outer loop in cmd/review-bot/main.go. Consider a named constant to avoid magic numbers and make the retry policy explicit.

sonnet-review-bot commented on pull request rodin/review-bot#48 2026-05-08 02:20:03 +00:00
fix: retry on transient LLM response body truncation

[MINOR] TestComplete_ContentLengthMismatch does not verify that exactly 2 attempts were made (unlike TestComplete_RetryOnBodyReadError). Without this check, the test would pass even if retry was not triggered (e.g., if the httptest server ignores the Content-Length override). Consider adding if attempts != 2 { t.Errorf(...) }.

sonnet-review-bot commented on pull request rodin/review-bot#45 2026-05-05 06:16:50 +00:00
fix: repair unescaped quotes in LLM JSON responses

[NIT] The comment above repairJSON says it will "pick the LAST" valid closing quote to maximize content, but findClosingQuote actually returns the FIRST candidate that yields valid continuation. Align the comment with the implementation or adjust the selection to match the described strategy.

sonnet-review-bot approved rodin/review-bot#45 2026-05-05 06:01:55 +00:00
fix: repair unescaped quotes in LLM JSON responses

Code Review — repairJSON fallback

sonnet-review-bot suggested changes for rodin/review-bot#45 2026-05-04 21:37:19 +00:00
fix: repair unescaped quotes in LLM JSON responses

Review: fix: repair unescaped quotes in LLM JSON responses

sonnet-review-bot commented on pull request rodin/review-bot#45 2026-05-04 21:36:24 +00:00
fix: repair unescaped quotes in LLM JSON responses

test

sonnet-review-bot commented on pull request rodin/review-bot#45 2026-05-04 21:36:20 +00:00
fix: repair unescaped quotes in LLM JSON responses

test

sonnet-review-bot suggested changes for rodin/review-bot#44 2026-05-03 04:07:45 +00:00
ci: fix reviewer models — sonnet uses Anthropic, gpt uses GPT-5

Gpt5-mini Review

sonnet-review-bot commented on pull request rodin/review-bot#44 2026-05-03 04:07:45 +00:00
ci: fix reviewer models — sonnet uses Anthropic, gpt uses GPT-5

[MAJOR] The matrix entry 'gpt41' sets token_secret: SONNET_REVIEW_TOKEN. This is likely incorrect — gpt-related entries should probably use GPT_REVIEW_TOKEN (or their own appropriate secret). Using the wrong token will cause that matrix job to authenticate as the wrong reviewer and may fail or operate under unexpected permissions.

sonnet-review-bot commented on pull request rodin/review-bot#44 2026-05-03 04:07:45 +00:00
ci: fix reviewer models — sonnet uses Anthropic, gpt uses GPT-5

[MINOR] You've added provider: anthropic and base_url for the 'sonnet' entry which is good. Make sure the review-bot supports Anthropic-style auth/headers when LLM_PROVIDER=anthropic and that a single LLM_API_KEY secret is valid for both Anthropic and OpenAI endpoints (or provide separate secrets if needed).

sonnet-review-bot commented on pull request rodin/review-bot#44 2026-05-03 04:07:45 +00:00
ci: fix reviewer models — sonnet uses Anthropic, gpt uses GPT-5

[MAJOR] The matrix entry 'gpt41-mini' sets token_secret: SONNET_REVIEW_TOKEN. As above, this seems inconsistent with the 'gpt' naming and should likely reference GPT_REVIEW_TOKEN or another correct secret.

sonnet-review-bot commented on pull request rodin/review-bot#44 2026-05-03 04:07:45 +00:00
ci: fix reviewer models — sonnet uses Anthropic, gpt uses GPT-5

[MAJOR] The matrix entry 'gpt5-mini' sets token_secret: SONNET_REVIEW_TOKEN. This duplicates the same token as 'sonnet' and likely should use GPT_REVIEW_TOKEN (or a dedicated token). Confirm intended reviewer tokens per matrix entry.

sonnet-review-bot commented on pull request rodin/review-bot#44 2026-05-03 04:07:45 +00:00
ci: fix reviewer models — sonnet uses Anthropic, gpt uses GPT-5

[NIT] SYSTEM_PROMPT_FILE is set from matrix.system_prompt_file; for entries that don't define it the variable will be empty — that's probably fine, but consider documenting that behavior in the PR body or adding an explicit null/empty entry for clarity.

sonnet-review-bot commented on pull request rodin/review-bot#44 2026-05-03 04:06:52 +00:00
ci: fix reviewer models — sonnet uses Anthropic, gpt uses GPT-5

[MINOR] The gpt41, gpt5-mini, and gpt41-mini matrix entries all use token_secret: SONNET_REVIEW_TOKEN rather than their own dedicated secrets or GPT_REVIEW_TOKEN. This appears intentional (sharing a token for auxiliary reviewers) but is worth confirming — if the Anthropic token is used to call an OpenAI endpoint it may fail depending on how the HAI proxy validates tokens.