Skip to content

Fix false negatives#31

Closed
cameronyule wants to merge 8 commits into
mainfrom
fix-false-negatives
Closed

Fix false negatives#31
cameronyule wants to merge 8 commits into
mainfrom
fix-false-negatives

Conversation

@cameronyule

@cameronyule cameronyule commented Jul 19, 2025

Copy link
Copy Markdown
Owner

The point of opening this PR was to test the Gemini Code Assist GitHub integration. I will keep it updated and eventually land the changes I need to make (fixing the false negatives) but for now it's WIP.

Reading the Playwright documentation, I realised I wasn't using the
BrowserContext feature which "allows creating isolated non-persistent
browser contexts … which don't write any browsing data to disk."
@cameronyule cameronyule linked an issue Jul 19, 2025 that may be closed by this pull request
gemini-code-assist[bot]

This comment was marked as outdated.

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

Copy link
Copy Markdown

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 aims to fix false negatives in bookmark processing. It introduces several changes: reducing concurrency, improving liveness checks in the headless browser to correctly handle HTTP error codes, and adding more logging for redirects. The test suite has been updated to reflect changes in the code structure. The review focuses on potential issues with performance, code clarity, and test coverage.

Comment thread src/bookmark_processor/main.py Outdated
Comment thread src/bookmark_processor/tasks/liveness.py
Comment thread src/bookmark_processor/main.py Outdated
@cameronyule
cameronyule force-pushed the fix-false-negatives branch from a81a785 to 4d0e9d9 Compare July 19, 2025 18:20
Co-authored-by: aider (gemini/gemini-2.5-flash) <aider@aider.chat>
@cameronyule
cameronyule force-pushed the fix-false-negatives branch from 4d0e9d9 to 0b97cf7 Compare July 19, 2025 18:35

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

Copy link
Copy Markdown

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 improves the liveness checks by using Playwright browser contexts and by handling HTTP error status codes in the headless browser check. The tests have been updated accordingly. There is a performance issue with using a Prefect task as a Playwright event handler, which could lead to excessive task creation. I've provided a suggestion to convert it to a regular function.

Comment thread src/bookmark_processor/tasks/liveness.py Outdated
cameronyule and others added 3 commits July 20, 2025 16:06
As identified by gemini-code-assist PR review.
Co-authored-by: aider (gemini/gemini-2.5-pro) <aider@aider.chat>
@cameronyule

Copy link
Copy Markdown
Owner Author

/gemini review

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

Copy link
Copy Markdown

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 refactors the headless browser check to use Playwright's BrowserContext for better isolation and correctly treats HTTP 4xx/5xx status codes as failures, which should help fix false positives. The accompanying tests have been updated and expanded to cover these changes.

Overall, the changes are well-implemented and the new tests are thorough. I've left a couple of suggestions to improve code clarity and test maintainability.

Comment thread src/bookmark_processor/tasks/liveness.py Outdated
Comment thread tests/integration/test_liveness_integration.py Outdated
cameronyule and others added 2 commits July 20, 2025 16:44
As suggested by Google Code Assist PR review.

Co-authored-by: aider (gemini/gemini-2.5-pro) <aider@aider.chat>
As suggested by Gemini Code Assist PR review.
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.

Liveness check false negatives

1 participant