Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Comments

Close side panel

fix: extract_gemini_web_search_count#418

Open
zachary822 wants to merge 3 commits intoPostHog:masterPostHog/posthog-python:masterfrom
zachary822:fix_extract_gemini_web_search_countzachary822/posthog-python:fix_extract_gemini_web_search_countCopy head branch name to clipboard
Open

fix: extract_gemini_web_search_count#418
zachary822 wants to merge 3 commits intoPostHog:masterPostHog/posthog-python:masterfrom
zachary822:fix_extract_gemini_web_search_countzachary822/posthog-python:fix_extract_gemini_web_search_countCopy head branch name to clipboard

Conversation

@zachary822
Copy link

extract_gemini_web_search_count errors if response.candidates is None

@zachary822 zachary822 changed the title Fix extract_gemini_web_search_count fix: extract_gemini_web_search_count Jan 31, 2026
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

1 file reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Comment on lines +1166 to +1169
def test_none_candidates_no_web_search(mock_client, mock_google_genai_client):
"""Test that response with candidates=None does not crash web search extraction."""

mock_response = MagicMock()
Copy link
Contributor

Choose a reason for hiding this comment

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

[P3] Prefer parameterizing the web-search “no candidates” cases

This new test covers candidates=None, but it duplicates the structure of the existing “no web search” tests nearby; given the repo preference for parameterized tests, it’d be simpler to combine the “no candidates” variants (e.g. candidates=None and candidates=[]) into one @pytest.mark.parametrize case table so new edge cases don’t keep adding near-identical blocks.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix With AI
This is a comment left during a code review.
Path: posthog/test/ai/gemini/test_gemini.py
Line: 1166:1169

Comment:
[P3] Prefer parameterizing the web-search “no candidates” cases

This new test covers `candidates=None`, but it duplicates the structure of the existing “no web search” tests nearby; given the repo preference for parameterized tests, it’d be simpler to combine the “no candidates” variants (e.g. `candidates=None` and `candidates=[]`) into one `@pytest.mark.parametrize` case table so new edge cases don’t keep adding near-identical blocks.

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.

@rafaeelaudibert
Copy link
Member

We've updated our release process. We require sampo now. Please rebase on master and check README to understand what should be done.

@zachary822
Copy link
Author

We've updated our release process. We require sampo now. Please rebase on master and check README to understand what should be done.

I added the file sampo generated.

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

Morty Proxy This is a proxified and sanitized view of the page, visit original site.