[f] VER-268: Add web search tool to Stage 3: In-depth Analysis#24
[f] VER-268: Add web search tool to Stage 3: In-depth Analysis#24quancao-ea merged 4 commits intomainfrom
Conversation
- Implement two-step processing with Google Search tool integration - Add Pydantic models for robust output validation (stage_3_models.py) - Store grounding metadata from web searches in database - Optimize validation flow: validate with Pydantic first, fallback to schema restructuring
Summary of ChangesHello @quancao-ea, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the 'In-depth Analysis' (Stage 3) of the processing pipeline by integrating a web search tool for the Gemini model and implementing a robust two-stage validation and structuring mechanism for its output. It also introduces Pydantic models for schema validation and ensures that grounding metadata from the web search is captured and stored. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds Pydantic Stage 3 output models and a two-stage Stage3Executor flow (search-enabled analysis then validation/structuring). Stage3Executor.run now returns (response_dict, grounding_metadata) which is propagated into Supabase updates. Adds Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Caller as process_snippet
participant Exec as Stage3Executor
participant GenAI as Google GenAI + Search
participant Pyd as Pydantic (Stage3Output)
participant DB as Supabase
Caller->>Exec: run(gemini_key, model_name, audio_file, metadata)
activate Exec
Exec->>GenAI: analyze_with_search(audio, prompts)
GenAI-->>Exec: analysis_text + grounding_metadata
Exec->>Pyd: validate(analysis_text)
alt Valid per Stage3Output
Pyd-->>Exec: parsed_output
else Invalid
Exec->>GenAI: structure_with_schema(schema, analysis_text)
GenAI-->>Exec: structured_output
end
Exec-->>Caller: (output_dict, grounding_metadata)
deactivate Exec
Caller->>DB: update_snippet(..., grounding_metadata, status, error_message)
DB-->>Caller: update result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Pylint (3.3.8)src/processing_pipeline/stage_3.py************* Module .pylintrc ... [truncated 16229 characters] ... nvention", 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 |
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed everything up to 14e5925 in 2 minutes and 2 seconds. Click for details.
- Reviewed
697lines of code in7files - Skipped
0files when reviewing. - Skipped posting
7draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. prompts/Stage_3_output_schema.json:18
- Draft comment:
New 'is_convertible' field added. Ensure that all producers and consumers of the JSON schema properly handle this field (both when present and absent). - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to ensure that all producers and consumers of the JSON schema handle the new field correctly. This falls under the rule of not asking the author to ensure behavior is intended or tested. It doesn't provide a specific suggestion or point out a specific issue.
2. requirements.txt:11
- Draft comment:
Removed 'google-generativeai' dependency. Verify that all functionality is now provided by 'google-genai' to avoid any regression. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to verify functionality after a dependency change, which violates the rule against asking for confirmation or verification. It does not provide a specific suggestion or point out a specific issue.
3. src/processing_pipeline/stage_3.py:160
- Draft comment:
Stage3Executor.run now returns a tuple (response, grounding_metadata). Confirm that downstream functions properly destructure and use both elements. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is asking the PR author to confirm that downstream functions properly destructure and use both elements of a returned tuple. This falls under the category of asking the author to ensure behavior is intended or to double-check things, which is against the rules.
4. src/processing_pipeline/stage_3_models.py:1
- Draft comment:
Pydantic models for Stage 3 output are comprehensive. Consider adding example values in Field descriptions to improve documentation and clarity. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%None
5. src/processing_pipeline/stage_4.py:210
- Draft comment:
Stage4Executor.run uses GeminiModel.GEMINI_2_5_PRO while other parts (e.g., Stage3Executor) use GEMINI_FLASH_LATEST. Verify if this version difference is intentional. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
6. src/processing_pipeline/supabase_utils.py:198
- Draft comment:
The update_snippet method now includes a new 'grounding_metadata' parameter. Ensure the database schema and downstream consumers are updated to handle this field. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to ensure that the database schema and downstream consumers are updated to handle a new parameter. This falls under the rule of not asking the author to ensure things are tested or verified, which is not allowed.
7. tests/processing_pipeline/test_stage_3.py:164
- Draft comment:
Tests now expect Stage3Executor.run to return a tuple with grounding metadata. Confirm that all test fixtures and assertions reflect the updated response structure. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to confirm that all test fixtures and assertions reflect the updated response structure. This falls under the rule of not asking the author to confirm or ensure things. It doesn't provide a specific suggestion or point out a specific issue.
Workflow ID: wflow_0HTVbvGb9OszSJmV
You can customize 
| """ | ||
| try: | ||
| print("Attempting to validate response with Pydantic model...") | ||
| start_idx = response_text.find("{") |
There was a problem hiding this comment.
JSON extraction in __validate_with_pydantic uses simple substring search (find and rfind). Consider a more robust extraction (e.g., regex or stricter parsing) in case the response contains extra text.
| start_idx = response_text.find("{") | |
| import re; match = re.search(r'{.*}', response_text, re.DOTALL); start_idx = match.start() if match else -1 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
Thank you for this contribution. The introduction of a two-stage processing logic with Pydantic validation and a fallback schema-based structuring is a solid strategy to improve the reliability of the Gemini model's output. The addition of the web search tool is also a great enhancement for the in-depth analysis stage.
I've identified a critical issue regarding safe dictionary access and a medium-severity suggestion to make the JSON parsing more robust. Please see my detailed comments below.
| start_idx = response_text.find("{") | ||
| end_idx = response_text.rfind("}") |
There was a problem hiding this comment.
Extracting the JSON object by finding the first { and the last } is a bit brittle. This approach can fail if the model's response includes { or } characters in explanatory text outside the main JSON object, or within string values inside the JSON itself.
A more robust method would be to use a regular expression to first look for a JSON object enclosed in markdown code fences (e.g., ```json ... ```), which is a common pattern for language models. If that pattern isn't found, you can then fall back to the current find/rfind logic. This would make the parsing more resilient to variations in the model's output format.
Here is an example of how you could implement this, which would require adding import re at the top of the file:
import re
# ... inside __validate_with_pydantic
json_str = None
# Try to find JSON in a markdown block
match = re.search(r"```json\s*(\{.*?\})\s*```", response_text, re.DOTALL)
if match:
json_str = match.group(1)
else:
# Fallback to finding the first and last brace
start_idx = response_text.find("{")
end_idx = response_text.rfind("}")
if start_idx != -1 and end_idx != -1:
json_str = response_text[start_idx : end_idx + 1]
if not json_str:
print("No JSON object found in the response.")
return None
try:
parsed = Stage3Output.model_validate_json(json_str)
# ...
except ValidationError as e:
# ...
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/processing_pipeline/stage_4.py (1)
276-280: Consolidate Gemini API key usage
In src/processing_pipeline/stage_4.py, therunmethod (line 205) usesGOOGLE_GEMINI_PAID_KEYwhile__ensure_json_format(line 276) usesGOOGLE_GEMINI_KEY. No README entry or other modules distinguish a paid‐tier key. Either unify both paths toGOOGLE_GEMINI_KEYor document and configureGOOGLE_GEMINI_PAID_KEYexplicitly.
🧹 Nitpick comments (5)
tests/processing_pipeline/test_stage_3.py (1)
164-168: Add negative-path tests for structuring failure.Please add tests where:
- Stage3Executor.run’s step 2 returns parsed with "is_convertible": false → process_snippet should set status "Error".
- Step 2 omits "is_convertible" (schema drift) → confirm error handling doesn’t crash and status is set.
Also applies to: 234-236, 246-251, 312-324
src/processing_pipeline/stage_3.py (3)
272-280: Update the return type in the docstring.Method returns a tuple
(response: dict, grounding_metadata), not justdict.- Returns: - dict: Structured and validated analysis output + Returns: + tuple[dict, object]: (Structured and validated analysis output, grounding metadata)
428-431: Makeis_convertibleaccess resilient.Even with schema requiring it, a defensive
.getavoids crashes on provider drift.- if not parsed_response["is_convertible"]: + if not parsed_response.get("is_convertible", False): raise ValueError("[Stage 3] The response from Gemini could not be converted to the required schema.")
351-353: Tidy long exception messages (ruff TRY003).Consider shorter messages or custom exception types/constants to satisfy TRY003.
Also applies to: 425-426, 429-429, 282-282
src/processing_pipeline/stage_3_models.py (1)
25-29: Optional: Add range constraints and remove unused import.
- Add
ge=0, le=100toEmotionalToneItem.intensityto match the description and schema.Literalimport is unused; remove it.Example:
-from typing import Literal -from pydantic import BaseModel, Field +from pydantic import BaseModel, Field @@ class EmotionalToneItem(BaseModel): emotion: EmotionText - intensity: int = Field(description="Intensity of the emotion, ranging from 0 to 100") + intensity: int = Field(ge=0, le=100, description="Intensity of the emotion, ranging from 0 to 100") evidence: EmotionEvidence explanation: EmotionExplanationAlso applies to: 100-105, 1-2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
prompts/Stage_3_output_schema.json(1 hunks)requirements.txt(0 hunks)src/processing_pipeline/stage_3.py(7 hunks)src/processing_pipeline/stage_3_models.py(1 hunks)src/processing_pipeline/stage_4.py(2 hunks)src/processing_pipeline/supabase_utils.py(2 hunks)tests/processing_pipeline/test_stage_3.py(6 hunks)
💤 Files with no reviewable changes (1)
- requirements.txt
🧰 Additional context used
🧬 Code graph analysis (3)
tests/processing_pipeline/test_stage_3.py (1)
tests/processing_pipeline/test_stage_4.py (3)
test_process_snippet(275-362)mock_supabase_client(22-31)sample_snippet(112-219)
src/processing_pipeline/stage_3.py (3)
src/processing_pipeline/stage_3_models.py (1)
Stage3Output(133-151)src/processing_pipeline/stage_4.py (1)
run(198-272)src/processing_pipeline/gemini_2_5_pro_transcription_generator.py (1)
run(22-76)
src/processing_pipeline/stage_4.py (1)
src/processing_pipeline/constants.py (1)
GeminiModel(5-11)
🪛 Ruff (0.13.3)
src/processing_pipeline/stage_3.py
282-282: Avoid specifying long messages outside the exception class
(TRY003)
351-351: Avoid specifying long messages outside the exception class
(TRY003)
353-353: Avoid specifying long messages outside the exception class
(TRY003)
425-425: Avoid specifying long messages outside the exception class
(TRY003)
426-426: Avoid specifying long messages outside the exception class
(TRY003)
429-429: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (3)
src/processing_pipeline/stage_4.py (3)
13-13: LGTM!The
ThinkingConfigimport is correctly added and used in the fallback path at line 305.
299-299: Model change to GEMINI_FLASH_LATEST is appropriate for the fallback task.Using
GEMINI_FLASH_LATESTfor the simple JSON formatting fallback is a reasonable optimization compared to a more expensive model. This lighter model should handle the straightforward task of ensuring JSON validity while reducing costs and latency.
305-305: LGTM!Setting
thinking_budget=0for the JSON formatting task is appropriate. This simple task doesn't require the model's extended reasoning capabilities, and disabling it will improve response time and reduce token usage.
| "is_convertible": { | ||
| "type": "boolean", | ||
| "description": "Indicates whether the provided text can be converted into a valid JSON object according to the given schema. Only use when asked to convert provided text into a valid JSON object." | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Make is_convertible required to match Stage 3 code; add numeric bounds for consistency.
Stage 3 does parsed_response["is_convertible"] (direct indexing). If the field isn’t emitted (it’s optional in the schema), that will raise a KeyError. Add it to the root required list. Also, enforce numeric ranges declared in descriptions to align with Pydantic validations.
Apply:
@@
- "required": [
+ "required": [
"transcription",
"translation",
"title",
"summary",
"explanation",
"disinformation_categories",
"keywords_detected",
"language",
"context",
"confidence_scores",
"emotional_tone",
- "political_leaning"
+ "political_leaning",
+ "is_convertible"
],
@@
"confidence_scores": {
"type": "object",
"required": ["overall", "analysis", "categories"],
"properties": {
"overall": {
- "type": "integer",
+ "type": "integer",
+ "minimum": 0,
+ "maximum": 100,
"description": "Overall confidence score of the analysis, ranging from 0 to 100."
},
@@
"score": {
- "type": "integer",
+ "type": "integer",
+ "minimum": 0,
+ "maximum": 100,
"description": "Confidence score for this category, ranging from 0 to 100."
}
@@
"intensity": {
- "type": "integer",
+ "type": "integer",
+ "minimum": 0,
+ "maximum": 100,
"description": "Intensity of the emotion, ranging from 0 to 100."
},
@@
"score": {
- "type": "number",
+ "type": "number",
+ "minimum": -1.0,
+ "maximum": 1.0,
"description": "Political leaning score, ranging from -1.0 to 1.0."
},📝 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.
| "is_convertible": { | |
| "type": "boolean", | |
| "description": "Indicates whether the provided text can be converted into a valid JSON object according to the given schema. Only use when asked to convert provided text into a valid JSON object." | |
| }, | |
| "required": [ | |
| "transcription", | |
| "translation", | |
| "title", | |
| "summary", | |
| "explanation", | |
| "disinformation_categories", | |
| "keywords_detected", | |
| "language", | |
| "context", | |
| "confidence_scores", | |
| "emotional_tone", | |
| "political_leaning", | |
| "is_convertible" | |
| ], | |
| "properties": { | |
| "confidence_scores": { | |
| "type": "object", | |
| "required": ["overall", "analysis", "categories"], | |
| "properties": { | |
| "overall": { | |
| "type": "integer", | |
| "minimum": 0, | |
| "maximum": 100, | |
| "description": "Overall confidence score of the analysis, ranging from 0 to 100." | |
| }, | |
| "analysis": { | |
| /* existing schema */ | |
| }, | |
| "categories": { | |
| "type": "array", | |
| "items": { | |
| "type": "object", | |
| "properties": { | |
| "score": { | |
| "type": "integer", | |
| "minimum": 0, | |
| "maximum": 100, | |
| "description": "Confidence score for this category, ranging from 0 to 100." | |
| } | |
| } | |
| } | |
| } | |
| } | |
| }, | |
| "emotional_tone": { | |
| "type": "object", | |
| "properties": { | |
| "intensity": { | |
| "type": "integer", | |
| "minimum": 0, | |
| "maximum": 100, | |
| "description": "Intensity of the emotion, ranging from 0 to 100." | |
| } | |
| /* existing properties */ | |
| } | |
| }, | |
| "political_leaning": { | |
| "type": "number", | |
| "minimum": -1.0, | |
| "maximum": 1.0, | |
| "description": "Political leaning score, ranging from -1.0 to 1.0." | |
| }, | |
| "is_convertible": { | |
| "type": "boolean", | |
| "description": "Indicates whether the provided text can be converted into a valid JSON object according to the given schema. Only use when asked to convert provided text into a valid JSON object." | |
| } | |
| /* other properties */ | |
| } |
🤖 Prompt for AI Agents
In prompts/Stage_3_output_schema.json around lines 18-21, the is_convertible
boolean is currently optional but Stage 3 code indexes
parsed_response["is_convertible"], which will raise a KeyError if missing; add
"is_convertible" to the root "required" array so it is always emitted. Also, for
any numeric fields in this schema whose descriptions mention ranges, add the
appropriate "minimum" and/or "maximum" properties to their schemas to enforce
those bounds (so Pydantic validation matches the documented ranges).
| grounding_metadata = str(response.candidates[0].grounding_metadata) if response.candidates else None | ||
|
|
||
| if not response.text: | ||
| finish_reason = response.candidates[0].finish_reason | ||
| if finish_reason == FinishReason.MAX_TOKENS: | ||
| raise ValueError("The response from Gemini was too long and was cut off in step 1.") | ||
| print(f"Response finish reason: {finish_reason}") | ||
| raise ValueError("No response from Gemini in step 1.") |
There was a problem hiding this comment.
Guard access to candidates when response.text is empty.
Indexing response.candidates[0] can raise when candidates is empty. Guard it as you do elsewhere.
- if not response.text:
- finish_reason = response.candidates[0].finish_reason
+ if not response.text:
+ finish_reason = response.candidates[0].finish_reason if response.candidates else None
if finish_reason == FinishReason.MAX_TOKENS:
raise ValueError("The response from Gemini was too long and was cut off in step 1.")
print(f"Response finish reason: {finish_reason}")
raise ValueError("No response from Gemini in step 1.")📝 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.
| grounding_metadata = str(response.candidates[0].grounding_metadata) if response.candidates else None | |
| if not response.text: | |
| finish_reason = response.candidates[0].finish_reason | |
| if finish_reason == FinishReason.MAX_TOKENS: | |
| raise ValueError("The response from Gemini was too long and was cut off in step 1.") | |
| print(f"Response finish reason: {finish_reason}") | |
| raise ValueError("No response from Gemini in step 1.") | |
| grounding_metadata = str(response.candidates[0].grounding_metadata) if response.candidates else None | |
| if not response.text: | |
| finish_reason = response.candidates[0].finish_reason if response.candidates else None | |
| if finish_reason == FinishReason.MAX_TOKENS: | |
| raise ValueError("The response from Gemini was too long and was cut off in step 1.") | |
| print(f"Response finish reason: {finish_reason}") | |
| raise ValueError("No response from Gemini in step 1.") |
🧰 Tools
🪛 Ruff (0.13.3)
351-351: Avoid specifying long messages outside the exception class
(TRY003)
353-353: Avoid specifying long messages outside the exception class
(TRY003)
🤖 Prompt for AI Agents
In src/processing_pipeline/stage_3.py around lines 346 to 353, the code accesses
response.candidates[0] when response.text is empty which will raise if
candidates is empty; guard candidate access by checking if response.candidates
and len(response.candidates) > 0 before reading grounding_metadata or
finish_reason, use a safe fallback (e.g., grounding_metadata = None and
finish_reason = None or a descriptive default) and adjust the error handling to
reference the safe finish_reason when present, then raise the existing
ValueErrors accordingly.
| parsed = Stage3Output.model_validate_json(response_text[start_idx : end_idx + 1]) | ||
| print("Validation successful - returning structured output") | ||
| return parsed.model_dump() |
There was a problem hiding this comment.
Dump Pydantic with aliases to match schema (language.register).
Without by_alias=True, you’ll emit register_ instead of register, diverging from the schema and DB payload.
- return parsed.model_dump()
+ return parsed.model_dump(by_alias=True)📝 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.
| parsed = Stage3Output.model_validate_json(response_text[start_idx : end_idx + 1]) | |
| print("Validation successful - returning structured output") | |
| return parsed.model_dump() | |
| parsed = Stage3Output.model_validate_json(response_text[start_idx : end_idx + 1]) | |
| print("Validation successful - returning structured output") | |
| return parsed.model_dump(by_alias=True) |
🤖 Prompt for AI Agents
In src/processing_pipeline/stage_3.py around lines 375 to 377, the code
currently calls parsed.model_dump() which emits field names using Python
attribute names (e.g., register_) instead of the schema aliases (e.g.,
register); update the dump call to include aliases by using
parsed.model_dump(by_alias=True) so the serialized output matches the expected
schema/DB payload.
| grounding_metadata, | ||
| status, |
There was a problem hiding this comment.
🧩 Analysis chain
Clear grounding_metadata on reset and ensure type consistency (dict vs string).
- Reset currently doesn’t clear grounding_metadata, leaving stale data after resets. Add it to the reset payload.
- Stage 3 builds grounding_metadata as a string; Stage 4 tests pass a dict. Please standardize on a JSON object (dict) and ensure the Supabase column type matches (jsonb recommended).
Patch to clear on reset:
@@ def reset_snippet(self, id):
- .update(
+ .update(
{
"transcription": None,
"translation": None,
"title": None,
"summary": None,
"explanation": None,
"disinformation_categories": None,
"keywords_detected": None,
"language": None,
"confidence_scores": None,
"emotional_tone": None,
"context": None,
"political_leaning": None,
+ "grounding_metadata": None,
"status": "New",
"error_message": None,
}
)Also confirm the DB column type:
- If json/jsonb: prefer passing a dict from both Stage 3 and Stage 4.
- If text: convert consistently to string in both places (less ideal for querying).
Also applies to: 220-221, 264-284
🏁 Script executed:
#!/bin/bash
# 1. Show reset_snippet implementation
rg -n "def reset_snippet" -C3 src/processing_pipeline/supabase_utils.py
# 2. Show update_snippet signature and usage of grounding_metadata
rg -n "def update_snippet" -C3 src/processing_pipeline/supabase_utils.py
# 3. Find where grounding_metadata is constructed in Stage 3
rg -n "grounding_metadata" -C3 src/processing_pipeline
# 4. Find tests or Stage 4 code passing grounding_metadata
rg -n "grounding_metadata" -C3 .Length of output: 30443
Clear grounding_metadata on reset and unify metadata type
- In
reset_snippet, add"grounding_metadata": Noneto the.update(...)payload so stale metadata is cleared. - Remove the
str()wrappers in Stage3Executor and Stage4Executor to return a native JSON object forgrounding_metadata, and confirm the Supabase column isjsonbfor querying.
🤖 Prompt for AI Agents
In src/processing_pipeline/supabase_utils.py around lines 200-201, the
reset_snippet update payload must explicitly clear grounding_metadata to avoid
stale data; add "grounding_metadata": None to the .update(...) payload. Also
remove any str() wrappers applied to grounding_metadata in Stage3Executor and
Stage4Executor so they return native Python dicts (JSON objects) instead of
strings, and ensure the Supabase column type is jsonb so queries operate on JSON
values.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
src/processing_pipeline/stage_3.py (4)
346-346: Guard against empty candidates list.While you check
if response.candidates, accessingresponse.candidates[0]can still raise anIndexErrorifcandidatesis an empty list. Use a safer pattern:- grounding_metadata = str(response.candidates[0].grounding_metadata) if response.candidates else None + grounding_metadata = str(response.candidates[0].grounding_metadata) if response.candidates and len(response.candidates) > 0 else NoneOr more idiomatically:
- grounding_metadata = str(response.candidates[0].grounding_metadata) if response.candidates else None + grounding_metadata = str(response.candidates[0].grounding_metadata) if (response.candidates and response.candidates[0]) else None
348-353: Guard candidate access when response.text is empty.Accessing
response.candidates[0].finish_reasonat line 349 will raise anIndexErrorifcandidatesis empty. Apply the same guard pattern used elsewhere:if not response.text: - finish_reason = response.candidates[0].finish_reason + finish_reason = response.candidates[0].finish_reason if response.candidates else None if finish_reason == FinishReason.MAX_TOKENS: raise ValueError("The response from Gemini was too long and was cut off in step 1.") print(f"Response finish reason: {finish_reason}") raise ValueError("No response from Gemini in step 1.")
368-373: JSON extraction is brittle.Extracting JSON using
find("{")andrfind("}")can fail if the response contains{or}in explanatory text or within string values. Consider a more robust extraction method.As suggested in a previous review, you could try extracting from markdown code blocks first:
try: print("Attempting to validate response with Pydantic model...") + import re + # Try to find JSON in markdown code block first + match = re.search(r"```json\s*(\{.*?\})\s*```", response_text, re.DOTALL) + if match: + json_str = match.group(1) + else: + # Fallback to find/rfind + start_idx = response_text.find("{") + end_idx = response_text.rfind("}") + if start_idx == -1 or end_idx == -1: + print("No JSON object found in the response.") + return None + json_str = response_text[start_idx : end_idx + 1] - start_idx = response_text.find("{") - end_idx = response_text.rfind("}") - - if start_idx == -1 or end_idx == -1: - print("No JSON object found in the response.") - return None - - parsed = Stage3Output.model_validate_json(response_text[start_idx : end_idx + 1]) + parsed = Stage3Output.model_validate_json(json_str)
375-377: Serialize with schema aliases.Calling
model_dump()withoutby_alias=Truewill emit Python field names (e.g.,register_) instead of the schema aliases (e.g.,register), causing the output to diverge from the expected schema and database payload.- return parsed.model_dump() + return parsed.model_dump(by_alias=True)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/processing_pipeline/stage_3.py(7 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/processing_pipeline/stage_3.py (4)
src/processing_pipeline/stage_3_models.py (1)
Stage3Output(133-151)src/processing_pipeline/stage_4.py (1)
run(198-272)src/processing_pipeline/gemini_2_5_pro_transcription_generator.py (1)
run(22-76)src/processing_pipeline/constants.py (1)
GeminiModel(5-11)
🪛 Ruff (0.13.3)
src/processing_pipeline/stage_3.py
282-282: Avoid specifying long messages outside the exception class
(TRY003)
351-351: Avoid specifying long messages outside the exception class
(TRY003)
353-353: Avoid specifying long messages outside the exception class
(TRY003)
425-425: Avoid specifying long messages outside the exception class
(TRY003)
426-426: Avoid specifying long messages outside the exception class
(TRY003)
429-429: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (6)
src/processing_pipeline/stage_3.py (6)
7-7: LGTM!The new imports are appropriate for the added functionality: ValidationError for Pydantic validation, File/GoogleSearch/Tool for web search integration, and Stage3Output for structured output validation.
Also applies to: 11-11, 14-14, 19-19, 28-28
72-108: LGTM!The addition of
grounding_metadataparameter is correctly integrated into the function signature and properly forwarded to the Supabase update call.
160-182: LGTM!The tuple unpacking correctly handles the new return signature from
Stage3Executor.run(), andgrounding_metadatais properly propagated to the Supabase update.
257-316: LGTM!The refactored two-stage processing flow is well-structured:
- Analyze with Google Search and obtain grounding metadata
- Attempt Pydantic validation for efficiency
- Fall back to schema-based restructuring if needed
The type annotations improve code clarity, and the try/finally block ensures proper cleanup of the uploaded audio file.
428-429: Use safe dictionary access foris_convertible.Directly accessing
parsed_response["is_convertible"]will raise aKeyErrorif the key is missing. Sinceis_convertibleis not a required field in the schema, a compliant response might omit it. Use.get()for safe access:- if not parsed_response.get("is_convertible"): + if not parsed_response.get("is_convertible"): raise ValueError("[Stage 3] The response from Gemini could not be converted to the required schema.")Wait, I see the code already uses
.get()at line 428. Let me re-check the past comment... The past comment says "direct dictionary access" but the current code shows.get(). This means the issue was already fixed. I should not flag this.Actually, looking at the existing comment more carefully, it says the issue is at "null-null" lines, which suggests it might have been fixed. Let me skip this one.
[actions]
Skip this - already using .get()
-->
433-456: LGTM!The extraction of safety settings into a reusable helper method reduces duplication and improves maintainability. The settings are consistent with those used in other stages.
| if not parsed_response: | ||
| finish_reason = response.candidates[0].finish_reason if response.candidates else None | ||
| if finish_reason == FinishReason.MAX_TOKENS: | ||
| raise ValueError("The response from Gemini was too long and was cut off in step 2.") | ||
| raise ValueError(f"No response from Gemini in step 2. Response finished with reason: {finish_reason}") |
There was a problem hiding this comment.
Guard candidate access when parsed_response is None.
At line 423, accessing response.candidates[0].finish_reason will raise an IndexError if candidates is empty:
if not parsed_response:
- finish_reason = response.candidates[0].finish_reason if response.candidates else None
+ finish_reason = (response.candidates[0].finish_reason
+ if (response.candidates and len(response.candidates) > 0)
+ else None)
if finish_reason == FinishReason.MAX_TOKENS:
raise ValueError("The response from Gemini was too long and was cut off in step 2.")
raise ValueError(f"No response from Gemini in step 2. Response finished with reason: {finish_reason}")📝 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.
| if not parsed_response: | |
| finish_reason = response.candidates[0].finish_reason if response.candidates else None | |
| if finish_reason == FinishReason.MAX_TOKENS: | |
| raise ValueError("The response from Gemini was too long and was cut off in step 2.") | |
| raise ValueError(f"No response from Gemini in step 2. Response finished with reason: {finish_reason}") | |
| if not parsed_response: | |
| finish_reason = (response.candidates[0].finish_reason | |
| if (response.candidates and len(response.candidates) > 0) | |
| else None) | |
| if finish_reason == FinishReason.MAX_TOKENS: | |
| raise ValueError("The response from Gemini was too long and was cut off in step 2.") | |
| raise ValueError(f"No response from Gemini in step 2. Response finished with reason: {finish_reason}") |
🧰 Tools
🪛 Ruff (0.13.3)
425-425: Avoid specifying long messages outside the exception class
(TRY003)
426-426: Avoid specifying long messages outside the exception class
(TRY003)
🤖 Prompt for AI Agents
In src/processing_pipeline/stage_3.py around lines 422–426, guard access to
response.candidates before indexing: compute finish_reason by first checking
that response.candidates is truthy and non-empty (e.g., use a length check or
next(iter(response.candidates), None)) and only then read finish_reason from
that candidate; if no candidate exists set finish_reason = None and retain the
existing error raising logic so you never do response.candidates[0] without
verifying a candidate is present.
2 participants
Important
Adds web search tool and Pydantic validation to Stage 3 processing, updates snippet handling, and modifies tests accordingly.
Stage3Executorfor audio analysis with Google Search.process_snippet()to handle new response structure and grounding metadata.Stage3Outputand related models instage_3_models.pyfor structured output validation.is_convertiblefield toStage_3_output_schema.jsonfor JSON conversion validation.google-generativeaifromrequirements.txt.test_stage_3.pyto mock new behavior and validate changes.This description was created by
for 14e5925. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Improvements
Chores
Tests