Skip to content

fix: allow internal whitespace in severity threshold marker for Gemini agent#640

Open
graycoldknight wants to merge 1 commit intoroborev-dev:mainfrom
graycoldknight:fix-gemini-severity-threshold-parsing
Open

fix: allow internal whitespace in severity threshold marker for Gemini agent#640
graycoldknight wants to merge 1 commit intoroborev-dev:mainfrom
graycoldknight:fix-gemini-severity-threshold-parsing

Conversation

@graycoldknight
Copy link
Copy Markdown

Fixes an issue where the Gemini model agent sometimes outputs the SEVERITY_THRESHOLD_MET marker with internal newlines (e.g., SEVERITY_THRESHOLD\n_MET), causing the IsMarkerOnlyOutput check to fail and incorrectly flag the review as a failure.

Evidence

We tested this by feeding SEVERITY_THRESHOLD\n_MET and SEVERITY_THRESHOLD\n\n_MET to IsMarkerOnlyOutput. Previously it failed, but with the stripping of internal whitespace (\n, \r, \t, ) before the final string comparison, it passes correctly. We've added these cases to the test suite in internal/config/config_test.go.

Fixes an issue where the Gemini model agent sometimes outputs the `SEVERITY_THRESHOLD_MET` marker with internal newlines (e.g., `SEVERITY_THRESHOLD\n_MET`), causing the `IsMarkerOnlyOutput` check to fail and incorrectly flag the review as a failure.

By stripping internal whitespace before the final string comparison, this parser quirk is resolved while still rejecting output that contains other prose.
@roborev-ci
Copy link
Copy Markdown

roborev-ci bot commented Apr 12, 2026

roborev: Combined Review (b91f72e)

Verdict: No medium-or-higher issues found; the change looks clean based on all reviews.

All three reviews found no issues at Medium, High, or Critical severity. The change appears to safely broaden IsMarkerOnlyOutput so the exact SEVERITY_THRESHOLD_MET marker tolerates internal ASCII whitespace, with regression coverage added for newline-splitting cases.


Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

@mariusvniekerk
Copy link
Copy Markdown
Collaborator

We should probably make this conditional on the gemini models

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.

2 participants