This PR fixes Gemini3 reasoning-only response handling and implements a comprehensive refactoring of the Antigravity provider to improve code quality, type safety, and error handling.#1
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis PR introduces GitHub Actions workflow automation for compliance and status checking, refactors the Antigravity provider to centralize utility functions and add comprehensive error handling, and creates new type definitions and validation modules for improved type safety. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas requiring extra attention:
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/bot-reply.yml (1)
555-588: Issue flow will fail because/tmp/bot-reply.mdis never created for issues
/tmp/bot-reply.mdis only copied in the “Save secure prompt from base branch” step, which runs only whenIS_PR == 'true'. However, the “Analyze comment and respond” step always does:envsubst "$VARS" < /tmp/bot-reply.md | opencode run --share -For
issue_commentevents on plain issues, this file will not exist and the job will fail.A minimal fix is to choose the prompt path based on whether the thread is a PR or an issue:
- if [ "${{ steps.context.outputs.IS_PR }}" = "true" ]; then - FULL_DIFF_PATH="$GITHUB_WORKSPACE/.mirrobot_files/first_review_diff.txt" - INCREMENTAL_DIFF_PATH="$GITHUB_WORKSPACE/.mirrobot_files/incremental_diff.txt" - LAST_REVIEWED_SHA="${{ steps.review_type.outputs.last_reviewed_sha }}" - else - FULL_DIFF_PATH="" - INCREMENTAL_DIFF_PATH="" - LAST_REVIEWED_SHA="" - fi - VARS='$THREAD_CONTEXT $NEW_COMMENT_AUTHOR $NEW_COMMENT_BODY $THREAD_NUMBER $GITHUB_REPOSITORY $THREAD_AUTHOR $PR_HEAD_SHA $IS_FIRST_REVIEW $FULL_DIFF_PATH $INCREMENTAL_DIFF_PATH $LAST_REVIEWED_SHA' - FULL_DIFF_PATH="$FULL_DIFF_PATH" INCREMENTAL_DIFF_PATH="$INCREMENTAL_DIFF_PATH" LAST_REVIEWED_SHA="$LAST_REVIEWED_SHA" envsubst "$VARS" < /tmp/bot-reply.md | opencode run --share - + if [ "${{ steps.context.outputs.IS_PR }}" = "true" ]; then + FULL_DIFF_PATH="$GITHUB_WORKSPACE/.mirrobot_files/first_review_diff.txt" + INCREMENTAL_DIFF_PATH="$GITHUB_WORKSPACE/.mirrobot_files/incremental_diff.txt" + LAST_REVIEWED_SHA="${{ steps.review_type.outputs.last_reviewed_sha }}" + PROMPT_PATH="/tmp/bot-reply.md" + else + FULL_DIFF_PATH="" + INCREMENTAL_DIFF_PATH="" + LAST_REVIEWED_SHA="" + PROMPT_PATH=".github/prompts/bot-reply.md" + fi + VARS='$THREAD_CONTEXT $NEW_COMMENT_AUTHOR $NEW_COMMENT_BODY $THREAD_NUMBER $GITHUB_REPOSITORY $THREAD_AUTHOR $PR_HEAD_SHA $IS_FIRST_REVIEW $FULL_DIFF_PATH $INCREMENTAL_DIFF_PATH $LAST_REVIEWED_SHA' + FULL_DIFF_PATH="$FULL_DIFF_PATH" INCREMENTAL_DIFF_PATH="$INCREMENTAL_DIFF_PATH" LAST_REVIEWED_SHA="$LAST_REVIEWED_SHA" envsubst "$VARS" < "$PROMPT_PATH" | opencode run --share -This keeps the secure base-branch prompt for PRs while making issue mentions work reliably.
🧹 Nitpick comments (13)
src/rotator_library/providers/antigravity_utils/request_helpers.py (1)
34-86: Consider lifting adjective/noun lists to module-level constants
generate_project_id()recreates relatively largeadjectivesandnounslists on every call. This is cheap but unnecessary and slightly obscures intent.You could define these lists once at module level (as tuples or constants) and reference them here, which improves clarity and avoids repeated allocations while keeping behavior identical.
src/rotator_library/providers/antigravity_utils/__init__.py (1)
22-29: Align__all__ordering with Ruff’s expectationsThe exports in
__all__are fine functionally, but Ruff reports RUF022 (__all__is not sorted). If you want clean lint runs, consider sorting this list alphabetically:-__all__ = [ - "generate_request_id", - "generate_session_id", - "generate_project_id", - "normalize_type_arrays", - "clean_claude_schema", - "recursively_parse_json_strings", -] +__all__ = [ + "clean_claude_schema", + "generate_project_id", + "generate_request_id", + "generate_session_id", + "normalize_type_arrays", + "recursively_parse_json_strings", +].github/prompts/compliance-check.md (1)
130-295: Minor markdownlint/style nits in compliance promptThe prompt content looks good; a few small markdown style issues are flagged by tooling:
- Several fenced code blocks lack a language spec (MD040). Adding
bash,markdown, ortextwhere appropriate will clear these.- One instance of “Single Turn Process” could be hyphenated (“Single‑turn process”) per LanguageTool, but that’s purely stylistic.
These are non-functional and can be fixed opportunistically if you care about a clean markdownlint run.
.github/workflows/compliance-check.yml (1)
352-357: Consider truncating at a line boundary for cleaner diffs.The current truncation may cut mid-line or mid-hunk. Consider using
head -cwith adjustment to the nearest newline for cleaner output..github/prompts/pr-review.md (1)
30-30: Missing space after###in heading.Some markdown renderers require a space after the hash marks for proper heading recognition.
-###STRICT RULES FOR COMMENT SIGNAL: +### STRICT RULES FOR COMMENT SIGNAL:src/rotator_library/providers/antigravity_validators.py (2)
190-191: 'description' requirement may be too strict.Not all tool definitions include a description. Consider making this a warning rather than a validation error, or making it optional.
10-12: Unused imports.
Unionis imported but not used in this module. The loggerlib_loggeris also defined but never used.src/rotator_library/providers/antigravity_provider.py (6)
414-426:__del__destructor is fragile; consider context manager.Python's
__del__is unreliable (may not run, may run during shutdown when objects are gone). Consider implementing__enter__/__exit__for explicit resource management, or usingatexit.register()for cleanup.The current implementation is acceptable but users should be aware that cache flushing isn't guaranteed.
1031-1034: Remove unnecessary f-string prefixes.These strings don't contain any placeholders, so the
fprefix is unnecessary.lib_logger.warning( - f"Skipping reasoning_content - no valid signature found. " - f"This may cause issues if thinking is enabled." + "Skipping reasoning_content - no valid signature found. " + "This may cause issues if thinking is enabled." )
2126-2173: Add exception chaining withraise ... fromfor better traceability.The re-raised exceptions lose the original exception context, making debugging harder. Use
raise ... from eto preserve the chain.- elif e.response.status_code == 401: - raise litellm.AuthenticationError(f"Invalid authentication credentials: {e}") + elif e.response.status_code == 401: + raise litellm.AuthenticationError(f"Invalid authentication credentials: {e}") from eApply similar changes to other exception raises in this block.
2216-2230: Duplicate error handling logic - consider extracting to helper.This error mapping is similar to
acompletion. Consider extracting to a helper method like_handle_http_error(e: httpx.HTTPStatusError)to reduce duplication and ensure consistent behavior.
2352-2368: Consider usinglogging.exceptionfor automatic stack traces.The
logging.errorcalls don't include stack traces. Usinglogging.exceptionwithin exception handlers automatically includes the traceback for easier debugging.- lib_logger.error(f"Token counting HTTP error {e.response.status_code}: {e}") + lib_logger.exception(f"Token counting HTTP error {e.response.status_code}")
248-255: Add exception chaining for better error context.try: value = int(value_str) return value except ValueError: - raise ValueError( + raise ValueError( f"Invalid integer value for {key}: '{value_str}'. " f"Expected a valid integer, got: {type(value_str).__name__}" - ) + ) from NoneUsing
from Noneexplicitly suppresses the original exception chain since the new message already explains the error context.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
.github/prompts/bot-reply.md(3 hunks).github/prompts/compliance-check.md(1 hunks).github/prompts/pr-review.md(7 hunks).github/workflows/bot-reply.yml(4 hunks).github/workflows/compliance-check.yml(1 hunks).github/workflows/status-check-init.yml(1 hunks).gitignore(1 hunks)src/rotator_library/providers/antigravity_provider.py(21 hunks)src/rotator_library/providers/antigravity_types.py(1 hunks)src/rotator_library/providers/antigravity_utils/__init__.py(1 hunks)src/rotator_library/providers/antigravity_utils/json_parsers.py(1 hunks)src/rotator_library/providers/antigravity_utils/request_helpers.py(1 hunks)src/rotator_library/providers/antigravity_utils/schema_transformers.py(1 hunks)src/rotator_library/providers/antigravity_validators.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/rotator_library/providers/antigravity_utils/__init__.py (3)
src/rotator_library/providers/antigravity_utils/request_helpers.py (3)
generate_request_id(13-20)generate_session_id(23-31)generate_project_id(34-87)src/rotator_library/providers/antigravity_utils/schema_transformers.py (2)
normalize_type_arrays(12-46)clean_claude_schema(49-121)src/rotator_library/providers/antigravity_utils/json_parsers.py (1)
recursively_parse_json_strings(16-108)
src/rotator_library/providers/antigravity_validators.py (1)
src/rotator_library/model_info_service.py (1)
max_output_tokens(171-172)
src/rotator_library/providers/antigravity_provider.py (3)
src/rotator_library/providers/antigravity_utils/request_helpers.py (3)
generate_request_id(13-20)generate_session_id(23-31)generate_project_id(34-87)src/rotator_library/providers/antigravity_utils/schema_transformers.py (2)
normalize_type_arrays(12-46)clean_claude_schema(49-121)src/rotator_library/providers/antigravity_utils/json_parsers.py (1)
recursively_parse_json_strings(16-108)
🪛 actionlint (1.7.9)
.github/workflows/compliance-check.yml
239-239: property "pr_info" is not defined in object type {setup: {conclusion: string; outcome: string; outputs: {token: string}}}
(expression)
🪛 LanguageTool
.github/prompts/compliance-check.md
[grammar] ~193-~193: Use a hyphen to join words.
Context: ...h file in the affected groups: Single Turn Process: 1. Focus on THIS FILE ON...
(QB_NEW_EN_HYPHEN)
.github/prompts/pr-review.md
[style] ~43-~43: You can shorten this phrase to improve clarity and avoid wordiness.
Context: ...uide. Trust linters for formatting. - Code that is acceptable: If a line or block of code is perfec...
(NNS_THAT_ARE_JJ)
[style] ~252-~252: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...re duplicates of existing discussion? - Which are trivial nits that can be ignored? -...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~262-~262: Consider using a different verb for a more formal wording.
Context: ...that a previously raised issue has been fixed (must reference the prior feedback). - ...
(FIX_RESOLVE)
🪛 markdownlint-cli2 (0.18.1)
.github/prompts/compliance-check.md
130-130: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
148-148: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
165-165: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
170-170: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
181-181: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
429-429: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
435-435: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
443-443: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
451-451: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
459-459: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
469-469: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
477-477: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
.github/prompts/pr-review.md
30-30: No space after hash on atx style heading
(MD018, no-missing-space-atx)
🪛 Ruff (0.14.6)
src/rotator_library/providers/antigravity_utils/json_parsers.py
60-60: Consider moving this statement to an else block
(TRY300)
src/rotator_library/providers/antigravity_utils/__init__.py
22-29: __all__ is not sorted
Apply an isort-style sorting to __all__
(RUF022)
src/rotator_library/providers/antigravity_validators.py
34-34: Avoid specifying long messages outside the exception class
(TRY003)
37-37: Avoid specifying long messages outside the exception class
(TRY003)
44-44: Avoid specifying long messages outside the exception class
(TRY003)
47-47: Avoid specifying long messages outside the exception class
(TRY003)
52-52: Avoid specifying long messages outside the exception class
(TRY003)
56-56: Avoid specifying long messages outside the exception class
(TRY003)
59-59: Avoid specifying long messages outside the exception class
(TRY003)
64-67: Avoid specifying long messages outside the exception class
(TRY003)
71-71: Avoid specifying long messages outside the exception class
(TRY003)
77-77: Avoid specifying long messages outside the exception class
(TRY003)
79-79: Avoid specifying long messages outside the exception class
(TRY003)
84-84: Avoid specifying long messages outside the exception class
(TRY003)
86-86: Avoid specifying long messages outside the exception class
(TRY003)
91-91: Avoid specifying long messages outside the exception class
(TRY003)
93-93: Avoid specifying long messages outside the exception class
(TRY003)
98-98: Avoid specifying long messages outside the exception class
(TRY003)
100-100: Avoid specifying long messages outside the exception class
(TRY003)
115-115: Avoid specifying long messages outside the exception class
(TRY003)
118-118: Avoid specifying long messages outside the exception class
(TRY003)
137-137: Avoid specifying long messages outside the exception class
(TRY003)
140-140: Avoid specifying long messages outside the exception class
(TRY003)
143-143: Avoid specifying long messages outside the exception class
(TRY003)
146-146: Avoid specifying long messages outside the exception class
(TRY003)
151-151: Avoid specifying long messages outside the exception class
(TRY003)
154-154: Avoid specifying long messages outside the exception class
(TRY003)
157-157: Avoid specifying long messages outside the exception class
(TRY003)
173-173: Avoid specifying long messages outside the exception class
(TRY003)
177-177: Avoid specifying long messages outside the exception class
(TRY003)
180-180: Avoid specifying long messages outside the exception class
(TRY003)
184-184: Avoid specifying long messages outside the exception class
(TRY003)
188-188: Avoid specifying long messages outside the exception class
(TRY003)
191-191: Avoid specifying long messages outside the exception class
(TRY003)
197-197: Avoid specifying long messages outside the exception class
(TRY003)
200-200: Avoid specifying long messages outside the exception class
(TRY003)
203-203: Avoid specifying long messages outside the exception class
(TRY003)
206-206: Avoid specifying long messages outside the exception class
(TRY003)
222-224: Avoid specifying long messages outside the exception class
(TRY003)
src/rotator_library/providers/antigravity_provider.py
250-250: Consider moving this statement to an else block
(TRY300)
252-255: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
252-255: Avoid specifying long messages outside the exception class
(TRY003)
362-364: Avoid specifying long messages outside the exception class
(TRY003)
366-368: Avoid specifying long messages outside the exception class
(TRY003)
419-419: Do not catch blind exception: Exception
(BLE001)
425-425: Do not catch blind exception: Exception
(BLE001)
1032-1032: f-string without any placeholders
Remove extraneous f prefix
(F541)
1033-1033: f-string without any placeholders
Remove extraneous f prefix
(F541)
1982-1982: Do not catch blind exception: Exception
(BLE001)
2138-2138: Do not catch blind exception: Exception
(BLE001)
2139-2139: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
2140-2142: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
2140-2142: Avoid specifying long messages outside the exception class
(TRY003)
2143-2145: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
2143-2145: Avoid specifying long messages outside the exception class
(TRY003)
2147-2147: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
2147-2147: Avoid specifying long messages outside the exception class
(TRY003)
2149-2149: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
2149-2149: Avoid specifying long messages outside the exception class
(TRY003)
2151-2151: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
2151-2151: Avoid specifying long messages outside the exception class
(TRY003)
2153-2153: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
2153-2153: Avoid specifying long messages outside the exception class
(TRY003)
2164-2164: Do not catch blind exception: Exception
(BLE001)
2165-2165: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
2166-2168: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
2166-2168: Avoid specifying long messages outside the exception class
(TRY003)
2169-2169: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
2169-2169: Avoid specifying long messages outside the exception class
(TRY003)
2171-2171: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
2171-2171: Avoid specifying long messages outside the exception class
(TRY003)
2173-2173: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
2173-2173: Avoid specifying long messages outside the exception class
(TRY003)
2219-2219: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
2219-2219: Avoid specifying long messages outside the exception class
(TRY003)
2221-2221: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
2221-2221: Avoid specifying long messages outside the exception class
(TRY003)
2223-2223: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
2223-2223: Avoid specifying long messages outside the exception class
(TRY003)
2225-2225: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
2225-2225: Avoid specifying long messages outside the exception class
(TRY003)
2226-2226: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
2226-2226: Avoid specifying long messages outside the exception class
(TRY003)
2228-2228: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
2228-2228: Avoid specifying long messages outside the exception class
(TRY003)
2230-2230: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
2230-2230: Avoid specifying long messages outside the exception class
(TRY003)
2353-2353: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
2356-2356: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
2359-2359: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
2362-2362: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
2365-2365: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
2367-2367: Do not catch blind exception: Exception
(BLE001)
2368-2368: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
🔇 Additional comments (11)
src/rotator_library/providers/antigravity_utils/json_parsers.py (2)
16-41: LGTM - Well-documented utility function.Clear docstring with practical examples. The recursive approach is appropriate for handling nested structures.
76-107: Malformed JSON auto-correction is appropriately cautious.The truncation heuristic using
rfindis reasonable for trailing garbage characters. Good that warnings are logged when corrections are applied. The recursive post-processing ensures nested structures are also handled..github/workflows/compliance-check.yml (2)
33-58: Workflow triggers and concurrency configured correctly.Good use of
pull_request_targetfor security, and the concurrency group handles multiple event sources appropriately.
540-586: Footer verification is well-implemented.The verification ensures the AI agent followed the output format. The
continue-on-error: trueis acceptable since this is a quality check rather than a blocking requirement..github/prompts/pr-review.md (2)
53-72: Clear agentic workflow instructions.The distinction between internal multi-turn analysis and single bundled output is well-articulated. This should help the AI produce thorough reviews without overwhelming users with intermediate output.
611-632: Well-structured error handling protocol.The two-level classification (fatal vs non-fatal) provides clear guidance for graceful degradation while ensuring critical failures halt execution appropriately.
src/rotator_library/providers/antigravity_types.py (3)
17-77: Well-structured Gemini API type definitions.Good use of
total=Falsefor optional fields andLiteraltypes for constrained values. The types align well with the Gemini API structure.
93-167: Response types follow OpenAI format correctly.Good separation between streaming and non-streaming response structures. The type definitions should improve type checking throughout the provider.
214-227: Utility types document complex internal state well.
ConversationStateandSanitizationResultprovide clear contracts for the thinking sanitization logic. Consider using these types in the method signatures for better type safety.src/rotator_library/providers/antigravity_provider.py (2)
38-45: Clean utility extraction.Good refactoring to centralize utility functions in the
antigravity_utilspackage. This improves code organization and testability.
360-368: Good fail-fast validation for TTL configuration.Validating TTL values early prevents subtle bugs from negative or zero cache durations.
| FILE_GROUPS_JSON: | | ||
| [ | ||
| { | ||
| "name": "GitHub Workflows", | ||
| "description": "When code changes affect the build or CI process, verify build.yml is updated with new steps, jobs, or release configurations. Check that code changes are reflected in build matrix, deploy steps, and CI/CD pipeline.", | ||
| "files": [ | ||
| ".github/workflows/build.yml", | ||
| ".github/workflows/cleanup.yml", | ||
| ] | ||
| }, | ||
| { | ||
| "name": "Documentation", | ||
| "description": "Ensure README.md and DOCUMENTATION.md reflect code changes. For new features (providers, configuration options, CLI changes), verify feature documentation exists in both files. For API endpoint changes, check that DOCUMENTATION.md is updated. The 'Deployment guide.md' should be updated for deployment-related changes.", | ||
| "files": [ | ||
| "README.md", | ||
| "DOCUMENTATION.md", | ||
| "Deployment guide.md", | ||
| "src/rotator_library/README.md" | ||
| ] | ||
| }, | ||
| { | ||
| "name": "Python Dependencies", | ||
| "description": "When requirements.txt changes, ensure all new dependencies are properly listed. When pyproject.toml in src/rotator_library changes, verify it's consistent with requirements.txt. No lockfile is required for this project, but verify dependency versions are compatible.", | ||
| "files": [ | ||
| "requirements.txt", | ||
| "src/rotator_library/pyproject.toml" | ||
| ] | ||
| }, | ||
| { | ||
| "name": "Provider Configuration", | ||
| "description": "When adding or modifying LLM providers in src/rotator_library/providers/, ensure the provider is documented in DOCUMENTATION.md and README.md. New providers should have corresponding model definitions in model_definitions.py if needed.", | ||
| "files": [ | ||
| "src/rotator_library/providers/**/*.py", | ||
| "src/rotator_library/model_definitions.py", | ||
| "src/rotator_library/provider_factory.py" | ||
| ] | ||
| }, | ||
| { | ||
| "name": "Proxy Application", | ||
| "description": "Changes to proxy_app endpoints, TUI launcher, or settings should be reflected in documentation. New CLI arguments should be documented in README.md Quick Start section.", | ||
| "files": [ | ||
| "src/proxy_app/main.py", | ||
| "src/proxy_app/launcher_tui.py", | ||
| "src/proxy_app/settings_tool.py", | ||
| "src/proxy_app/batch_manager.py", | ||
| "src/proxy_app/detailed_logger.py" | ||
| ] | ||
| } | ||
| ] |
There was a problem hiding this comment.
Invalid JSON: trailing commas will cause parse failures.
The FILE_GROUPS_JSON contains trailing commas which are not valid JSON syntax. This will cause jq parsing to fail at line 391.
"files": [
".github/workflows/build.yml",
- ".github/workflows/cleanup.yml",
+ ".github/workflows/cleanup.yml"
]
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| FILE_GROUPS_JSON: | | |
| [ | |
| { | |
| "name": "GitHub Workflows", | |
| "description": "When code changes affect the build or CI process, verify build.yml is updated with new steps, jobs, or release configurations. Check that code changes are reflected in build matrix, deploy steps, and CI/CD pipeline.", | |
| "files": [ | |
| ".github/workflows/build.yml", | |
| ".github/workflows/cleanup.yml", | |
| ] | |
| }, | |
| { | |
| "name": "Documentation", | |
| "description": "Ensure README.md and DOCUMENTATION.md reflect code changes. For new features (providers, configuration options, CLI changes), verify feature documentation exists in both files. For API endpoint changes, check that DOCUMENTATION.md is updated. The 'Deployment guide.md' should be updated for deployment-related changes.", | |
| "files": [ | |
| "README.md", | |
| "DOCUMENTATION.md", | |
| "Deployment guide.md", | |
| "src/rotator_library/README.md" | |
| ] | |
| }, | |
| { | |
| "name": "Python Dependencies", | |
| "description": "When requirements.txt changes, ensure all new dependencies are properly listed. When pyproject.toml in src/rotator_library changes, verify it's consistent with requirements.txt. No lockfile is required for this project, but verify dependency versions are compatible.", | |
| "files": [ | |
| "requirements.txt", | |
| "src/rotator_library/pyproject.toml" | |
| ] | |
| }, | |
| { | |
| "name": "Provider Configuration", | |
| "description": "When adding or modifying LLM providers in src/rotator_library/providers/, ensure the provider is documented in DOCUMENTATION.md and README.md. New providers should have corresponding model definitions in model_definitions.py if needed.", | |
| "files": [ | |
| "src/rotator_library/providers/**/*.py", | |
| "src/rotator_library/model_definitions.py", | |
| "src/rotator_library/provider_factory.py" | |
| ] | |
| }, | |
| { | |
| "name": "Proxy Application", | |
| "description": "Changes to proxy_app endpoints, TUI launcher, or settings should be reflected in documentation. New CLI arguments should be documented in README.md Quick Start section.", | |
| "files": [ | |
| "src/proxy_app/main.py", | |
| "src/proxy_app/launcher_tui.py", | |
| "src/proxy_app/settings_tool.py", | |
| "src/proxy_app/batch_manager.py", | |
| "src/proxy_app/detailed_logger.py" | |
| ] | |
| } | |
| ] | |
| FILE_GROUPS_JSON: | | |
| [ | |
| { | |
| "name": "GitHub Workflows", | |
| "description": "When code changes affect the build or CI process, verify build.yml is updated with new steps, jobs, or release configurations. Check that code changes are reflected in build matrix, deploy steps, and CI/CD pipeline.", | |
| "files": [ | |
| ".github/workflows/build.yml", | |
| ".github/workflows/cleanup.yml" | |
| ] | |
| }, | |
| { | |
| "name": "Documentation", | |
| "description": "Ensure README.md and DOCUMENTATION.md reflect code changes. For new features (providers, configuration options, CLI changes), verify feature documentation exists in both files. For API endpoint changes, check that DOCUMENTATION.md is updated. The 'Deployment guide.md' should be updated for deployment-related changes.", | |
| "files": [ | |
| "README.md", | |
| "DOCUMENTATION.md", | |
| "Deployment guide.md", | |
| "src/rotator_library/README.md" | |
| ] | |
| }, | |
| { | |
| "name": "Python Dependencies", | |
| "description": "When requirements.txt changes, ensure all new dependencies are properly listed. When pyproject.toml in src/rotator_library changes, verify it's consistent with requirements.txt. No lockfile is required for this project, but verify dependency versions are compatible.", | |
| "files": [ | |
| "requirements.txt", | |
| "src/rotator_library/pyproject.toml" | |
| ] | |
| }, | |
| { | |
| "name": "Provider Configuration", | |
| "description": "When adding or modifying LLM providers in src/rotator_library/providers/, ensure the provider is documented in DOCUMENTATION.md and README.md. New providers should have corresponding model definitions in model_definitions.py if needed.", | |
| "files": [ | |
| "src/rotator_library/providers/**/*.py", | |
| "src/rotator_library/model_definitions.py", | |
| "src/rotator_library/provider_factory.py" | |
| ] | |
| }, | |
| { | |
| "name": "Proxy Application", | |
| "description": "Changes to proxy_app endpoints, TUI launcher, or settings should be reflected in documentation. New CLI arguments should be documented in README.md Quick Start section.", | |
| "files": [ | |
| "src/proxy_app/main.py", | |
| "src/proxy_app/launcher_tui.py", | |
| "src/proxy_app/settings_tool.py", | |
| "src/proxy_app/batch_manager.py", | |
| "src/proxy_app/detailed_logger.py" | |
| ] | |
| } | |
| ] |
🤖 Prompt for AI Agents
.github/workflows/compliance-check.yml around lines 111 to 159: the
FILE_GROUPS_JSON block contains trailing commas in arrays/objects which produce
invalid JSON and break jq parsing; remove any trailing commas after the last
element in each "files" array and after the last object in the top-level array
so the string is valid JSON (ensure commas only appear between items, not after
the final item) and re-run a JSON linter to verify correctness.
| # Requested reviewers for mentions | ||
| reviewers=$(echo "$pr_json" | jq -r '.reviewRequests[]? | .login' | tr '\n' ' ') | ||
| mentions="@${{ steps.pr_info.outputs.pr_author }}" | ||
| if [ -n "$reviewers" ]; then | ||
| for reviewer in $reviewers; do | ||
| mentions="$mentions @$reviewer" | ||
| done | ||
| fi | ||
| echo "reviewer_mentions=$reviewers" >> $GITHUB_OUTPUT | ||
| echo "all_mentions=$mentions" >> $GITHUB_OUTPUT |
There was a problem hiding this comment.
Bug: Step references its own output before it's set.
Line 265 uses ${{ steps.pr_info.outputs.pr_author }} but this code runs within the pr_info step itself. The output pr_author hasn't been set yet at this point in execution.
# Requested reviewers for mentions
reviewers=$(echo "$pr_json" | jq -r '.reviewRequests[]? | .login' | tr '\n' ' ')
- mentions="@${{ steps.pr_info.outputs.pr_author }}"
+ pr_author=$(echo "$pr_json" | jq -r .author.login)
+ mentions="@$pr_author"
if [ -n "$reviewers" ]; then
for reviewer in $reviewers; do
mentions="$mentions @$reviewer"
done
fi🤖 Prompt for AI Agents
.github/workflows/compliance-check.yml lines 263-272: the step mistakenly
references `${{ steps.pr_info.outputs.pr_author }}` before that output is
created; replace that reference by extracting the PR author directly from the
local pr_json (e.g. parse pr_json with jq to get the author login into a shell
variable like pr_author=$(echo "$pr_json" | jq -r '.author.login')) or move the
mentions logic to after the step sets its outputs; then build mentions from that
local pr_author variable (not the step output) and write
reviewer_mentions/all_mentions to GITHUB_OUTPUT as before.
| elif isinstance(obj, str): | ||
| stripped = obj.strip() | ||
|
|
||
| # Check if string contains common escape sequences that need unescaping | ||
| # This handles cases where diff content or other text has literal \n instead of newlines | ||
| if '\\n' in obj or '\\t' in obj or '\\"' in obj or '\\\\' in obj: | ||
| try: | ||
| # Use json.loads with quotes to properly unescape the string | ||
| # This converts \n -> newline, \t -> tab, \" -> quote, etc. | ||
| unescaped = json.loads(f'"{obj}"') | ||
| lib_logger.debug( | ||
| f"[Antigravity] Unescaped string content: " | ||
| f"{len(obj) - len(unescaped)} chars changed" | ||
| ) | ||
| return unescaped | ||
| except (json.JSONDecodeError, ValueError): | ||
| # If unescaping fails, continue with original processing | ||
| pass |
There was a problem hiding this comment.
Logic ordering may prevent JSON parsing of escaped JSON strings.
If a string contains escape sequences AND is valid JSON (e.g., '[{\"key\": \"value\\nwith newline\"}]'), the unescaping on line 55 will convert it to a plain string and return early, preventing the JSON parsing logic below from running.
Consider restructuring to attempt JSON parsing first, then fall back to unescaping only for non-JSON strings:
elif isinstance(obj, str):
stripped = obj.strip()
+ # Try JSON parsing first for strings that look like JSON
+ if stripped and stripped[0] in ('{', '['):
+ # ... existing JSON parsing logic ...
+ pass
+
# Check if string contains common escape sequences that need unescaping
- if '\\n' in obj or '\\t' in obj or '\\"' in obj or '\\\\' in obj:
+ # Only unescape if not JSON-like
+ elif '\\n' in obj or '\\t' in obj or '\\"' in obj or '\\\\' in obj:Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Ruff (0.14.6)
60-60: Consider moving this statement to an else block
(TRY300)
| # Validate role | ||
| valid_roles = {'user', 'assistant', 'system'} | ||
| if message['role'] not in valid_roles: | ||
| raise ValueError( | ||
| f"Message at index {i} has invalid role '{message['role']}'. " | ||
| f"Must be one of: {', '.join(valid_roles)}" | ||
| ) |
There was a problem hiding this comment.
Missing "tool" role in allowed roles.
The valid_roles set excludes "tool" which is used for tool response messages in the OpenAI API format. This will cause validation failures when processing conversations with tool calls.
# Validate role
- valid_roles = {'user', 'assistant', 'system'}
+ valid_roles = {'user', 'assistant', 'system', 'tool'}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Validate role | |
| valid_roles = {'user', 'assistant', 'system'} | |
| if message['role'] not in valid_roles: | |
| raise ValueError( | |
| f"Message at index {i} has invalid role '{message['role']}'. " | |
| f"Must be one of: {', '.join(valid_roles)}" | |
| ) | |
| # Validate role | |
| valid_roles = {'user', 'assistant', 'system', 'tool'} | |
| if message['role'] not in valid_roles: | |
| raise ValueError( | |
| f"Message at index {i} has invalid role '{message['role']}'. " | |
| f"Must be one of: {', '.join(valid_roles)}" | |
| ) |
🧰 Tools
🪛 Ruff (0.14.6)
64-67: Avoid specifying long messages outside the exception class
(TRY003)
🤖 Prompt for AI Agents
In src/rotator_library/providers/antigravity_validators.py around lines 61 to
67, the valid_roles set omits the "tool" role causing valid tool response
messages to be rejected; update the validator to include "tool" in the allowed
roles (e.g., valid_roles = {'user', 'assistant', 'system', 'tool'}) so messages
with role "tool" pass validation, and optionally run tests or sample
conversations containing tool responses to verify no further validation issues
occur.
| # Validate content | ||
| if not isinstance(message['content'], str): | ||
| raise TypeError(f"Message content must be a string, got {type(message['content']).__name__}") |
There was a problem hiding this comment.
Content type validation is too restrictive.
The OpenAI API allows content to be:
- A string (simple text)
- A list of content parts (multimodal: text + images)
Nonefor assistant messages with tool_calls
# Validate content
- if not isinstance(message['content'], str):
- raise TypeError(f"Message content must be a string, got {type(message['content']).__name__}")
+ content = message['content']
+ if content is not None and not isinstance(content, (str, list)):
+ raise TypeError(f"Message content must be a string, list, or None, got {type(content).__name__}")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Validate content | |
| if not isinstance(message['content'], str): | |
| raise TypeError(f"Message content must be a string, got {type(message['content']).__name__}") | |
| # Validate content | |
| content = message['content'] | |
| if content is not None and not isinstance(content, (str, list)): | |
| raise TypeError(f"Message content must be a string, list, or None, got {type(content).__name__}") |
🧰 Tools
🪛 Ruff (0.14.6)
71-71: Avoid specifying long messages outside the exception class
(TRY003)
🤖 Prompt for AI Agents
In src/rotator_library/providers/antigravity_validators.py around lines 69-71,
the current validation rejects non-string message['content']; update it to
accept three cases allowed by the OpenAI API: a string, a list (multimodal
content parts), or None (for assistant messages with tool_calls). Replace the
strict isinstance(...) check with logic that: 1) allows None; 2) allows str; 3)
if it's a list, iterate and validate each element is either a str or a
dict/structure representing a content part (raise TypeError if any element is an
unexpected type); otherwise raise TypeError describing the allowed types.
| if reasoning_effort is not None: | ||
| valid_levels = {'low', 'medium', 'high'} | ||
| if reasoning_effort not in valid_levels: | ||
| raise ValueError( | ||
| f"reasoning_effort must be one of {', '.join(valid_levels)}, got '{reasoning_effort}'" | ||
| ) No newline at end of file |
There was a problem hiding this comment.
Missing "disable" option in valid reasoning effort levels.
The provider code at antigravity_provider.py line 855 handles reasoning_effort == "disable" to disable thinking. This validator should include "disable" as a valid option.
if reasoning_effort is not None:
- valid_levels = {'low', 'medium', 'high'}
+ valid_levels = {'low', 'medium', 'high', 'disable'}
if reasoning_effort not in valid_levels:🧰 Tools
🪛 Ruff (0.14.6)
222-224: Avoid specifying long messages outside the exception class
(TRY003)
🤖 Prompt for AI Agents
In src/rotator_library/providers/antigravity_validators.py around lines 219-224,
the validator's valid_levels set omits the "disable" option even though
antigravity_provider.py accepts reasoning_effort == "disable"; update the
valid_levels set to include "disable" (e.g., {'low','medium','high','disable'})
and ensure the ValueError message lists the new allowed values so "disable" is
accepted and reported correctly.
|
Already merged manually |
2 participants
Overview
This PR fixes Gemini3 reasoning-only response handling and implements a comprehensive refactoring of the Antigravity provider to improve code quality, type safety, and error handling.
Bug Fix
Gemini3 Reasoning-Only Response Handling
thoughtcontent (no text) were causing errorsRefactoring - 4 Phase Implementation
Phase 1: Utility Extraction ✅
antigravity_utils/module structurerequest_helpers.py,schema_transformers.py,json_parsers.pyPhase 2: Type Safety ✅
antigravity_types.pywith comprehensive TypedDict definitionsPhase 3: Input Validation ✅
antigravity_validators.pyfor parameter validationPhase 4: Error Handling ✅
Exceptionhandlers with specific httpx exception typesSecurity Improvements
randomtosecretsmodule for cryptographically secure random generationsecrets.randbelow(),secrets.choice(), andsecrets.token_hex()for ID generationSchema Handling Enhancements
normalize_type_arrays()clean_claude_schema()Files Changed
.gitignore- Added oauth_creds/, .roo/, and personal planning docssrc/rotator_library/providers/antigravity_provider.py- Main refactoringsrc/rotator_library/providers/antigravity_types.py- NEW: Type definitionssrc/rotator_library/providers/antigravity_validators.py- NEW: Input validationsrc/rotator_library/providers/antigravity_utils/- NEW: Utility module directory__init__.pyrequest_helpers.pyschema_transformers.pyjson_parsers.pyTesting
Benefits
✅ Better code organization: Separated concerns with utility modules
✅ Improved type safety: Comprehensive TypedDict definitions
✅ Better debugging: Specific error types with detailed messages
✅ Enhanced security: Cryptographically secure random generation
✅ Improved maintainability: Clear structure and better documentation
✅ Graceful degradation: Safe fallbacks for token counting and model discovery
Summary by CodeRabbit
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.