Skip to content

[f] VER-274: Allow snippets from stage 3 to skip analysis review from stage 4#30

Merged
quancao-ea merged 7 commits intomainfrom
features/allow-snippets-from-stage-3-to-skip-analysis-review-from-stage-4
Oct 28, 2025
Merged

[f] VER-274: Allow snippets from stage 3 to skip analysis review from stage 4#30
quancao-ea merged 7 commits intomainfrom
features/allow-snippets-from-stage-3-to-skip-analysis-review-from-stage-4

Conversation

@quancao-ea
Copy link
Copy Markdown
Collaborator

@quancao-ea quancao-ea commented Oct 28, 2025

Important

Add skip_review option to stage 3 processing, centralize safety settings, and enhance post-processing in the pipeline.

  • Behavior:
    • Add skip_review parameter to in_depth_analysis in stage_3.py to allow skipping stage 4 review.
    • Modify process_snippet in stage_3.py to mark snippets as "Processed" and trigger postprocess_snippet if skip_review is true.
    • Centralize safety settings in processing_utils.py and use in stage_3.py and stage_4.py.
  • Functions:
    • Add postprocess_snippet in processing_utils.py for label creation and embedding refresh.
    • Update update_snippet_in_supabase in stage_3.py to use gemini_response dict.
  • Tests:
    • Add tests for postprocess_snippet in test_processing_utils.py.
    • Add tests for process_snippet with skip_review in test_stage_3.py.
    • Add tests for process_snippet and analysis_review in test_stage_4.py.

This description was created by Ellipsis for c68f0a6. You can customize this summary. It will automatically update as commits are pushed.


Summary by CodeRabbit

  • New Features

    • Optional "skip review" pathway that marks items as Processed and triggers automated post-processing (batch label creation and embedding refresh).
  • Improvements

    • Centralized safety configuration for all analysis steps.
    • More robust handling of missing response fields and upgraded main analysis model for improved results.
  • Tests

    • Added unit tests covering post-processing and skip-review flows.

@linear
Copy link
Copy Markdown

linear Bot commented Oct 28, 2025

@gemini-code-assist
Copy link
Copy Markdown
Contributor

Summary of Changes

Hello @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 enhances the processing pipeline by introducing a configurable option to skip the manual analysis review stage (Stage 4) for snippets that have completed Stage 3 processing. This change streamlines the workflow for certain use cases, allowing for faster progression of snippets through the system. It also involves significant refactoring to centralize common utility functions and safety settings, improving code maintainability and consistency across different processing stages.

Highlights

  • Skip Analysis Review: Introduced a skip_review parameter in Stage 3 processing, allowing snippets to bypass the manual analysis review in Stage 4 and be marked as 'Processed' directly.
  • Refactored Post-processing Logic: Moved common post-processing steps, such as creating and assigning labels and deleting vector embeddings, into a new postprocess_snippet utility function in processing_utils.py. This function is now called conditionally in Stage 3 and consistently in Stage 4.
  • Centralized Safety Settings: Extracted the Gemini API safety settings into a shared get_safety_settings function in processing_utils.py, reducing redundancy and simplifying maintenance across Stage 3 and Stage 4.
  • Gemini Model Update: Updated the Gemini model used in Stage 3 from GEMINI_FLASH_LATEST to GEMINI_2_5_PRO for potentially improved analysis capabilities.
  • Test Coverage: Added a new test file test_processing_utils.py for the new utility functions and updated existing Stage 3 and Stage 4 tests to reflect the new skip_review parameter and refactored logic.
Using Gemini Code Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Oct 28, 2025

Note

Other AI code review bot(s) detected

CodeRabbit 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.

Walkthrough

Adds centralized post-processing utilities and integrates them into Stage 3/4; introduces a structured gemini_response payload and a skip_review flag that can auto-process snippets; centralizes Gemini safety settings via get_safety_settings() and updates tests to reflect these flows.

Changes

Cohort / File(s) Summary
Post-processing utilities
src/processing_pipeline/processing_utils.py
Adds three @optional_task functions: create_new_label_and_assign_to_snippet, delete_vector_embedding_of_snippet, and postprocess_snippet; imports optional_task.
Stage 3 core changes
src/processing_pipeline/stage_3.py
Reworks update_snippet_in_supabase to accept gemini_response + grounding_metadata, adds skip_review to process_snippet and in_depth_analysis, renames audio_fileuploaded_audio_file, replaces inline safety with get_safety_settings(), switches main model to GEMINI_2_5_PRO, and calls postprocess_snippet when skip_review=True.
Stage 4 cleanup
src/processing_pipeline/stage_4.py
Removes local label/embedding helper functions, replaces inline SafetySetting blocks with get_safety_settings(), and delegates post-processing to postprocess_snippet.
New unit tests
tests/processing_pipeline/test_processing_utils.py
Adds tests (mocks) for the three processing_utils functions including normal and error cases.
Stage 3 tests updated
tests/processing_pipeline/test_stage_3.py
Adjusts tests for gemini_response parameter and skip_review behavior; patches postprocess_snippet expectations.
Stage 4 tests updated
tests/processing_pipeline/test_stage_4.py
Removes imports of removed helpers, adds get_audio_file_by_id mocking and audio_file fields, and asserts postprocess_snippet is invoked where appropriate.
Deployment parameter change
src/processing_pipeline/main.py
Adds skip_review=True to in-depth analysis deployment parameters list.

Sequence Diagram(s)

sequenceDiagram
    participant Scheduler as Scheduler/User
    participant Stage3 as Stage 3 Executor
    participant Utils as processing_utils
    participant Supabase as Supabase
    participant Stage4 as Stage 4 Executor

    Note over Stage3,Utils: Auto-process path (skip_review = true)
    Scheduler->>Stage3: process_snippet(snippet, skip_review=True)
    Stage3->>Supabase: update_snippet(snippet_id, gemini_response, status="Processed")
    Stage3->>Utils: postprocess_snippet(supabase_client, snippet_id, disinformation_categories)
    Utils->>Supabase: create label & assign (loop per category)
    Utils->>Supabase: delete_vector_embedding_of_snippet(snippet_id)

    Note over Stage3,Stage4: Manual review path (skip_review = false)
    Scheduler->>Stage3: process_snippet(snippet, skip_review=False)
    Stage3->>Supabase: update_snippet(snippet_id, gemini_response, status="Ready for review")
    Stage4->>Stage4: review/ensure JSON (uses get_safety_settings())
    Stage4->>Supabase: update_snippet(..., status="Ready")
    Stage4->>Utils: postprocess_snippet(supabase_client, snippet_id, disinformation_categories)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Pay attention to:
    • All call sites and tests expecting the previous flat field list now receiving gemini_response.
    • Correct propagation and usage of the skip_review flag across Stage 3 → Stage 4.
    • Mapping of previous inline SafetySetting structures to get_safety_settings() for Gemini API compatibility.
    • Post-processing side effects (label creation and embedding deletion) centralized in processing_utils.

Possibly related PRs

Suggested reviewers

  • nhphong

Poem

🐰 Hops with a clipboard, neat and quick —
Labels sewn, embeddings kicked,
Skip the review or let it stay,
Gemini hums a brighter day.
Postprocess done, the pipeline's slick.

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Out of Scope Changes Check ⚠️ Warning The PR includes one notable out-of-scope change: switching the Gemini model from GEMINI_FLASH_LATEST to GEMINI_2_5_PRO in the main analysis path of stage_3.py. This model change is not mentioned in the VER-274 linked issue objectives and represents a significant behavioral change affecting model performance and API costs that should be addressed separately or explicitly documented. Other changes such as safety settings centralization, postprocessing implementation, and the update_snippet_in_supabase refactoring are directly related to or necessary for the skip_review feature and remain in scope. Remove the Gemini model switch from this PR or create a separate issue (VER-xxx) documenting and justifying the model change. If the model change is intentional and required for the feature, add it to the linked issue requirements and update the PR description to explicitly call this out as a significant change. Verify that all other changes align with the VER-274 objectives.
Docstring Coverage ⚠️ Warning Docstring coverage is 75.51% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The PR title "[f] VER-274: Allow snippets from stage 3 to skip analysis review from stage 4" clearly and specifically summarizes the primary change in the changeset. It accurately reflects the main feature being implemented—enabling a skip_review mechanism to bypass Stage 4 analysis. The title is concise, readable, and directly related to the objective of the pull request. Someone scanning commit history would immediately understand the core change being introduced.
Linked Issues Check ✅ Passed The code changes comprehensively address all objectives from VER-274. The skip_review parameter is added to process_snippet and in_depth_analysis [stage_3.py, main.py], allowing snippets to bypass Stage 4 review. When skip_review is true, snippets receive "Processed" status instead of "Ready for review" and trigger the new postprocess_snippet function [stage_3.py]. The postprocess_snippet, create_new_label_and_assign_to_snippet, and delete_vector_embedding_of_snippet functions are implemented in processing_utils.py [processing_utils.py]. Safety settings are centralized via get_safety_settings in processing_utils.py [processing_utils.py, stage_3.py, stage_4.py]. Comprehensive tests are added covering postprocessing behavior and the skip_review flow [test_processing_utils.py, test_stage_3.py, test_stage_4.py].
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch features/allow-snippets-from-stage-3-to-skip-analysis-review-from-stage-4

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 (4.0.1)
src/processing_pipeline/stage_3.py

************* Module .pylintrc
.pylintrc:1:0: F0011: error while parsing the configuration: File contains no section headers.
file: '.pylintrc', line: 1
'disable=C0116\n' (config-parse-error)
[
{
"type": "convention",
"module": "src.processing_pipeline.stage_3",
"obj": "",
"line": 37,
"column": 0,
"endLine": null,
"endColumn": null,
"path": "src/processing_pipeline/stage_3.py",
"symbol": "line-too-long",
"message": "Line too long (180/100)",
"message-id": "C0301"
},
{
"type": "convention",
"module": "src.processing_pipeline.stage_3",
"obj": "",
"line": 115,
"column": 0,
"endLine": null,
"endColumn": null,
"path": "src/processing_pipeline/stage_3.py",
"symbol": "line-too-long",
"message": "Line too long (119/100)",
"message-id": "C0301"
},
{
"type": "convention",
"module": "src.

... [truncated 17339 characters] ...

nvention",
"module": "src.processing_pipeline.stage_3",
"obj": "",
"line": 5,
"column": 0,
"endLine": 5,
"endColumn": 11,
"path": "src/processing_pipeline/stage_3.py",
"symbol": "wrong-import-order",
"message": "standard import "json" should be placed before third party import "google.genai"",
"message-id": "C0411"
},
{
"type": "convention",
"module": "src.processing_pipeline.stage_3",
"obj": "",
"line": 10,
"column": 0,
"endLine": 17,
"endColumn": 1,
"path": "src/processing_pipeline/stage_3.py",
"symbol": "ungrouped-imports",
"message": "Imports from package google are not grouped",
"message-id": "C0412"
}
]


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a feature to allow snippets from stage 3 to bypass the stage 4 analysis review, which is a significant workflow enhancement. The implementation is solid, and I appreciate the excellent refactoring work to centralize shared logic into processing_utils.py and update the tests accordingly. I've provided a few suggestions to further improve code maintainability and correctness, mainly around reducing code duplication and ensuring docstrings are up-to-date. Overall, this is a high-quality contribution.

Comment thread src/processing_pipeline/stage_3.py Outdated
Comment on lines +193 to +210
mock_supabase_client.update_snippet.assert_called_once_with(
id=sample_snippet["id"],
transcription=mock_gemini_response["transcription"],
translation=mock_gemini_response["translation"],
title=mock_gemini_response["title"],
summary=mock_gemini_response["summary"],
explanation=mock_gemini_response["explanation"],
disinformation_categories=mock_gemini_response["disinformation_categories"],
keywords_detected=mock_gemini_response["keywords_detected"],
language=mock_gemini_response["language"],
confidence_scores=mock_gemini_response["confidence_scores"],
emotional_tone=mock_gemini_response["emotional_tone"],
context=mock_gemini_response["context"],
political_leaning=mock_gemini_response["political_leaning"],
grounding_metadata="test_grounding_metadata",
status="Processed",
error_message=None,
)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

This assert_called_once_with block is almost an exact copy of the one in test_process_snippet_skip_review_false. This duplication makes the tests harder to maintain. Consider refactoring by creating a base dictionary of expected arguments and only changing the status field for each test case. This will make your tests DRY (Don't Repeat Yourself) and easier to read.

Copy link
Copy Markdown
Contributor

@ellipsis-dev ellipsis-dev Bot left a comment

Choose a reason for hiding this comment

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

Caution

Changes requested ❌

Reviewed everything up to 097b266 in 1 minute and 54 seconds. Click for details.
  • Reviewed 932 lines of code in 6 files
  • Skipped 0 files when reviewing.
  • Skipped posting 7 draft 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. src/processing_pipeline/stage_3.py:143
  • Draft comment:
    Good use of the new skip_review parameter to conditionally update the snippet status. Consider using a logging framework instead of print for production-level traceability.
  • Reason this comment was not posted:
    Comment was on unchanged code.
2. src/processing_pipeline/stage_3.py:327
  • Draft comment:
    Replacing class method __get_safety_settings with global get_safety_settings is clear. Verify that all changes in safety settings usage across stages maintain consistent behavior.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% The comment is asking the PR author to verify that the changes maintain consistent behavior, which violates the rule against asking for confirmation or verification of behavior. It does not provide a specific suggestion or point out a specific issue with the code.
3. src/processing_pipeline/stage_4.py:92
  • Draft comment:
    Refactoring label creation and vector embedding deletion to postprocess_snippet cleans up stage 4 code. Ensure that any potential errors from postprocess_snippet are handled appropriately.
  • 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.
4. tests/processing_pipeline/test_processing_utils.py:45
  • Draft comment:
    Tests for processing_utils functions are comprehensive. Ensure that exception scenarios from the optional_task decorator are also covered in integration tests.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% The comment is asking the author to ensure that exception scenarios are covered in integration tests. This is a general request for testing and does not provide specific guidance or suggestions. It violates the rule against asking the author to ensure that the change is tested.
5. tests/processing_pipeline/test_stage_3.py:153
  • Draft comment:
    Tests now pass the skip_review parameter to process_snippet. This improves coverage. Consider asserting the exact error message in error tests for better clarity.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% The first part is purely informative and not actionable. The second part about error messages is a matter of testing style - using mock.ANY for error messages is often intentional to avoid brittle tests. The current approach in the tests seems reasonable and consistent. Could the suggestion about error messages actually improve test quality in some cases? Maybe some error messages are important to validate exactly? While exact error message assertions can be valuable in some cases, the current tests already use a mix of exact and flexible assertions appropriately based on the test's needs. The comment doesn't identify any specific cases where the current approach is problematic. The comment should be deleted as it combines a non-actionable observation with a general suggestion that doesn't identify specific issues with the current implementation.
6. tests/processing_pipeline/test_stage_4.py:243
  • Draft comment:
    Tests for prepare_snippet_for_review check proper field extraction; ensure additional edge cases (e.g. missing fields) are covered as in later tests.
  • Reason this comment was not posted:
    Comment was on unchanged code.
7. tests/processing_pipeline/test_stage_4.py:700
  • Draft comment:
    Test 'test_analysis_review_with_specific_ids' swallows exceptions. Consider explicitly asserting expected error outcomes rather than using a bare try/except pass.
  • 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.

Workflow ID: wflow_SVnNMi7fUTj6MJ0G

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

spanish_label_text = label["spanish"]

# Create the label
label = supabase_client.create_new_label(english_label_text, spanish_label_text)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Consider renaming the local variable in create_new_label_and_assign_to_snippet (line 41) to avoid shadowing the input parameter label. For example, use created_label instead.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
tests/processing_pipeline/test_stage_3.py (1)

317-328: Remove unused mock variable.

The mock_process variable is patched but never used in assertions. Either verify calls on this mock or remove the patch if it's not needed for this test.

Apply this diff to utilize the mock:

         with patch("os.remove"), patch("time.sleep") as mock_sleep, patch(
             "processing_pipeline.stage_3.process_snippet"
         ) as mock_process:

             try:
                 in_depth_analysis(snippet_ids=None, repeat=True, skip_review=True)
             except StopIteration:
                 pass  # Ignore StopIteration as we expect it

             assert mock_supabase_client.get_a_new_snippet_and_reserve_it.call_count >= 1
             assert mock_sleep.call_count >= 1
             mock_sleep.assert_called_with(60)  # Should sleep when no new snippets found
+            # Verify process_snippet was called for the first snippet
+            assert mock_process.call_count >= 1

Based on static analysis hints.

📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between e8a5190 and 097b266.

📒 Files selected for processing (6)
  • src/processing_pipeline/processing_utils.py (2 hunks)
  • src/processing_pipeline/stage_3.py (8 hunks)
  • src/processing_pipeline/stage_4.py (4 hunks)
  • tests/processing_pipeline/test_processing_utils.py (1 hunks)
  • tests/processing_pipeline/test_stage_3.py (10 hunks)
  • tests/processing_pipeline/test_stage_4.py (13 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
src/processing_pipeline/processing_utils.py (2)
src/utils.py (1)
  • optional_task (6-40)
src/processing_pipeline/supabase_utils.py (2)
  • create_new_label (324-342)
  • assign_label_to_snippet (344-359)
tests/processing_pipeline/test_processing_utils.py (2)
src/processing_pipeline/processing_utils.py (2)
  • create_new_label_and_assign_to_snippet (36-44)
  • postprocess_snippet (53-59)
src/processing_pipeline/supabase_utils.py (2)
  • create_new_label (324-342)
  • assign_label_to_snippet (344-359)
tests/processing_pipeline/test_stage_4.py (2)
src/processing_pipeline/supabase_utils.py (2)
  • get_audio_file_by_id (37-39)
  • submit_snippet_review (240-262)
src/processing_pipeline/stage_4.py (3)
  • prepare_snippet_for_review (30-67)
  • process_snippet (93-129)
  • analysis_review (154-185)
tests/processing_pipeline/test_stage_3.py (4)
tests/processing_pipeline/test_processing_utils.py (1)
  • mock_supabase_client (12-18)
tests/processing_pipeline/test_stage_4.py (2)
  • mock_supabase_client (20-35)
  • sample_snippet (116-225)
src/processing_pipeline/stage_3.py (3)
  • update_snippet_in_supabase (73-98)
  • process_snippet (143-183)
  • in_depth_analysis (187-239)
src/processing_pipeline/supabase_utils.py (1)
  • update_snippet (185-229)
src/processing_pipeline/stage_4.py (1)
src/processing_pipeline/processing_utils.py (2)
  • get_safety_settings (10-32)
  • postprocess_snippet (53-59)
src/processing_pipeline/stage_3.py (4)
src/processing_pipeline/processing_utils.py (2)
  • get_safety_settings (10-32)
  • postprocess_snippet (53-59)
src/processing_pipeline/supabase_utils.py (1)
  • update_snippet (185-229)
src/processing_pipeline/stage_4.py (2)
  • process_snippet (93-129)
  • run (195-248)
src/processing_pipeline/constants.py (1)
  • GeminiModel (5-11)
🪛 Ruff (0.14.1)
tests/processing_pipeline/test_processing_utils.py

40-40: Do not assert blind exception: Exception

(B017)

tests/processing_pipeline/test_stage_3.py

319-319: Local variable mock_process is assigned to but never used

Remove assignment to unused variable mock_process

(F841)

src/processing_pipeline/stage_3.py

181-181: Do not catch blind exception: Exception

(BLE001)


345-345: Avoid specifying long messages outside the exception class

(TRY003)


421-421: Avoid specifying long messages outside the exception class

(TRY003)

🔇 Additional comments (2)
src/processing_pipeline/stage_3.py (2)

150-155: Verify model switch aligns with requirements.

The primary analysis model has been changed from GEMINI_FLASH_LATEST to GEMINI_2_5_PRO. This is a significant change that will affect:

  • Analysis quality and capability
  • Processing cost
  • Response time

Ensure this model upgrade is intentional and has been validated for the Stage 3 analysis requirements.


187-187: Verify default skip_review=True is appropriate.

The in_depth_analysis flow defaults to skip_review=True, meaning snippets will bypass Stage 4 review by default. Confirm this default aligns with the desired workflow, as it changes the existing behavior where snippets would normally proceed to review.

Copy link
Copy Markdown
Contributor

@ellipsis-dev ellipsis-dev Bot left a comment

Choose a reason for hiding this comment

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

Caution

Changes requested ❌

Reviewed 547b7b6 in 1 minute and 29 seconds. Click for details.
  • Reviewed 26 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 draft 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. src/processing_pipeline/main.py:67
  • Draft comment:
    Added 'skip_review' flag to deployment parameters. Verify that its default behavior (True) is consistently expected downstream.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None

Workflow ID: wflow_pYX3MG086Q9CMWID

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.


@optional_flow(name="Stage 3: In-depth Analysis", log_prints=True, task_runner=ConcurrentTaskRunner)
def in_depth_analysis(snippet_ids, repeat):
def in_depth_analysis(snippet_ids, skip_review, repeat):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Parameter order changed in in_depth_analysis: now defined as (snippet_ids, skip_review, repeat) instead of (snippet_ids, repeat, skip_review=True). This could break positional calls; consider retaining default values and order for backward compatibility.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

♻️ Duplicate comments (2)
src/processing_pipeline/stage_3.py (2)

157-177: Refactor to eliminate code duplication.

Both branches call update_snippet_in_supabase with identical parameters except for status. The conditional logic and postprocessing call can be separated, reducing repetition and improving maintainability.

Apply this diff to consolidate the duplicate code:

-        if skip_review:
-            update_snippet_in_supabase(
-                supabase_client=supabase_client,
-                snippet_id=snippet["id"],
-                gemini_response=response,
-                grounding_metadata=grounding_metadata,
-                status="Processed",
-                error_message=None,
-            )
-
-            postprocess_snippet(supabase_client, snippet["id"], response["disinformation_categories"])
-
-        else:
-            update_snippet_in_supabase(
-                supabase_client=supabase_client,
-                snippet_id=snippet["id"],
-                gemini_response=response,
-                grounding_metadata=grounding_metadata,
-                status="Ready for review",
-                error_message=None,
-            )
-
+        status = "Processed" if skip_review else "Ready for review"
+        update_snippet_in_supabase(
+            supabase_client=supabase_client,
+            snippet_id=snippet["id"],
+            gemini_response=response,
+            grounding_metadata=grounding_metadata,
+            status=status,
+            error_message=None,
+        )
+
+        if skip_review:
+            postprocess_snippet(supabase_client, snippet["id"], response["disinformation_categories"])
+

310-311: Correct the decorator ordering.

The @classmethod decorator should be applied directly to the method (innermost position) before other decorators like @optional_task. The current order may cause the task decorator to wrap the unbound method incorrectly.

Apply this diff to fix the decorator order:

-    @optional_task(log_prints=True, retries=3)
-    @classmethod
+    @classmethod
+    @optional_task(log_prints=True, retries=3)
🧹 Nitpick comments (1)
src/processing_pipeline/stage_3.py (1)

143-143: Document the skip_review parameter.

Add a docstring or inline comment explaining when skip_review should be True vs False, as this is a critical control flow parameter that determines whether snippets bypass human review.

📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 097b266 and 547b7b6.

📒 Files selected for processing (2)
  • src/processing_pipeline/main.py (1 hunks)
  • src/processing_pipeline/stage_3.py (8 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/processing_pipeline/stage_3.py (3)
src/processing_pipeline/processing_utils.py (2)
  • get_safety_settings (10-32)
  • postprocess_snippet (53-59)
src/processing_pipeline/supabase_utils.py (1)
  • update_snippet (185-229)
src/processing_pipeline/constants.py (1)
  • GeminiModel (5-11)
🪛 Ruff (0.14.1)
src/processing_pipeline/stage_3.py

181-181: Do not catch blind exception: Exception

(BLE001)


345-345: Avoid specifying long messages outside the exception class

(TRY003)


421-421: Avoid specifying long messages outside the exception class

(TRY003)

🔇 Additional comments (6)
src/processing_pipeline/stage_3.py (5)

19-22: LGTM! Safety settings and postprocessing centralized.

The imports now include get_safety_settings and postprocess_snippet from processing_utils, which consolidates reusable utilities and eliminates inline duplication across stages.


335-335: LGTM! Safety settings now centralized.

Using get_safety_settings() eliminates inline safety configuration duplication and ensures consistent safety behavior across all Gemini API calls.

Also applies to: 411-411


342-348: LGTM! Safer finish reason handling.

The code now safely checks if response.candidates exists before accessing finish_reason, preventing potential index errors when the response is empty or malformed.

Also applies to: 419-423


73-98: All callers correctly pass the gemini_response dictionary with the expected structure.

Verification confirms:

  • Stage3Output Pydantic model defines all 12 required fields: transcription, translation, title, summary, explanation, disinformation_categories, keywords_detected, language, confidence_scores, emotional_tone, context, and political_leaning
  • Both paths in Stage3Executor.run() return fully-populated dictionaries from the validated model or schema
  • All three call sites (lines 158, 170, and test at line 131) pass the response variable correctly with the new signature
  • The response dictionary is also successfully accessed at line 167 (response["disinformation_categories"]), confirming all expected keys are present

152-152: Verify the model upgrade validation and latency impact on processing SLAs.

The model was upgraded from GEMINI_FLASH_LATEST to GEMINI_2_5_PRO in commit 61eb805. Gemini 2.5 Pro is significantly more expensive than Flash and positioned for maximum quality and deep reasoning, while Flash is optimized for high-throughput, low-latency, cost-sensitive production workloads.

For this audio processing pipeline:

  • Cost: API expenses will increase per token processed
  • Latency: Pro has higher per-token pricing and may incur higher latency overhead compared to Flash
  • Fit: The upgrade appears to optimize for reasoning quality, but stage 3 processes disinformation detection on snippets—clarify whether Pro's advanced reasoning capabilities justify the latency/cost tradeoff for this workload

No validation evidence (performance tests, latency benchmarks, or SLA updates) is present in the code history. Ensure this change was intentional and validated for the expected improvements. Also note: the log message at line 158 still says "Gemini 2.5 Flash" despite passing GEMINI_2_5_PRO—update logging to match the actual model.

src/processing_pipeline/main.py (1)

67-67: Confirm the default skip_review=True is intentional for production.

Setting skip_review=True in the deployment parameters means all snippets processed through this deployment will bypass Stage 4 human review and be marked as "Processed" immediately. This represents a significant workflow change from manual review to automated processing.

Ensure this default aligns with your operational requirements and consider:

  • Whether all content should skip review or only certain categories
  • If this should be configurable per deployment or based on snippet characteristics
  • Documentation/communication to reviewers about the workflow change

…ueries

Consolidates the conditional logic for updating snippet status by computing the status upfront and making a single database update call, reducing code duplication and improving maintainability.
Copy link
Copy Markdown
Contributor

@ellipsis-dev ellipsis-dev Bot left a comment

Choose a reason for hiding this comment

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

Important

Looks good to me! 👍

Reviewed c68f0a6 in 47 seconds. Click for details.
  • Reviewed 43 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 draft 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. src/processing_pipeline/stage_3.py:157
  • Draft comment:
    Good refactor: combining the update call into one unified call with status set based on skip_review improves clarity. Ensure that this change does not inadvertently affect downstream logic that might depend on separate update calls.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
2. src/processing_pipeline/stage_3.py:167
  • Draft comment:
    Ensure that calling postprocess_snippet only when skip_review is true is intentional and that it safely handles the expected response structure (e.g., response['disinformation_categories'] exists).
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None

Workflow ID: wflow_eowXEkbHaSEVEqJ6

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

@PublicDataWorks PublicDataWorks deleted a comment from gemini-code-assist Bot Oct 28, 2025
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

♻️ Duplicate comments (1)
src/processing_pipeline/stage_3.py (1)

301-302: Critical: Fix decorator order.

The @classmethod decorator must be the innermost decorator (applied directly to the function) before other decorators like @optional_task. The current order can cause incorrect method binding.

Apply this diff:

-    @optional_task(log_prints=True, retries=3)
-    @classmethod
+    @classmethod
+    @optional_task(log_prints=True, retries=3)

Based on coding guidelines

🧹 Nitpick comments (1)
src/processing_pipeline/stage_3.py (1)

402-414: LGTM: Consistent safety settings and improved error handling.

The centralized get_safety_settings() usage and defensive finish_reason extraction are good improvements.

Minor: Consider extracting the error messages at lines 336, 412, and 414 into custom exception classes for better maintainability, per Ruff's TRY003 guideline:

class GeminiResponseError(Exception):
    """Raised when Gemini fails to return a valid response."""
    pass

# Usage:
if finish_reason == FinishReason.MAX_TOKENS:
    raise GeminiResponseError("Response too long - cut off in step 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 547b7b6 and c68f0a6.

📒 Files selected for processing (1)
  • src/processing_pipeline/stage_3.py (8 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/processing_pipeline/stage_3.py (3)
src/processing_pipeline/processing_utils.py (2)
  • get_safety_settings (10-32)
  • postprocess_snippet (53-59)
src/processing_pipeline/supabase_utils.py (2)
  • update_snippet (185-229)
  • set_snippet_status (69-79)
src/processing_pipeline/constants.py (1)
  • GeminiModel (5-11)
🪛 Ruff (0.14.1)
src/processing_pipeline/stage_3.py

172-172: Do not catch blind exception: Exception

(BLE001)


336-336: Avoid specifying long messages outside the exception class

(TRY003)


412-412: Avoid specifying long messages outside the exception class

(TRY003)

🔇 Additional comments (7)
src/processing_pipeline/stage_3.py (7)

19-22: LGTM: Clean centralization of utilities.

The import changes properly support the new skip_review feature and centralize safety settings management.


320-341: LGTM: Improved error handling and centralized safety settings.

The use of get_safety_settings() centralizes configuration, and the defensive checks for missing response.candidates improve robustness against API edge cases.


148-148: LGTM: Better handling of non-ASCII content.

Setting ensure_ascii=False improves log readability for international characters in metadata.


172-174: Acceptable: Broad exception handling for top-level orchestration.

While static analysis flags the broad Exception catch, it's appropriate here for top-level error handling that ensures any failure is logged and recorded in the database rather than crashing the pipeline.


178-178: The review comment is incorrect—parameter order does not break these callers.

All calls to in_depth_analysis in the codebase use keyword arguments (e.g., in_depth_analysis(snippet_ids=None, repeat=False, skip_review=True)), where parameter names matter, not position. Parameter order changes have zero impact on keyword argument calls. No positional argument callers exist.

Likely an incorrect or invalid review comment.


143-170: Verify the model upgrade is intentional and accounts for increased costs.

The skip_review logic correctly sets status to "Processed" and triggers post-processing when enabled. However, the model change at line 152 from GEMINI_FLASH_LATEST to GEMINI_2_5_PRO carries significant cost implications: Gemini 2.5 Pro is positioned for maximum accuracy and reasoning performance but at substantially higher cost (reported at ~$1–$15 per 1M tokens), while Gemini Flash is the cost-efficient model (reported at ~$0.10–$0.30 per 1M input tokens). Confirm this upgrade was intentional and whether the performance benefit justifies the cost increase for this workload.


73-98: Refactoring verified as correct - no issues found.

The gemini_response dict structure is properly validated and converted to dict format at both code paths:

  • __validate_with_pydantic returns parsed.model_dump() (dict)
  • __structure_with_schema returns dict from Gemini JSON response

All Stage3Output model fields match the dict keys accessed in update_snippet_in_supabase. Both call sites have been updated to pass the dict format correctly.

@nhphong nhphong self-requested a review October 28, 2025 12:16
@quancao-ea quancao-ea merged commit bba24e1 into main Oct 28, 2025
2 checks passed
@coderabbitai coderabbitai Bot mentioned this pull request Feb 14, 2026
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