Skip to content

feat: Add production-ready improvements, new examples, and fix Release Please - #5

Merged
rishitank merged 11 commits into
mainfrom
feature/upgrade-deps-and-examples
Feb 6, 2026
Merged

rishitank merged 11 commits into
mainfrom
feature/upgrade-deps-and-examples

Conversation

@rishitank

Copy link
Copy Markdown
Owner

Summary

This PR adds production-ready improvements based on deep research findings, 4 new example scripts, and fixes the Release Please workflow.

Changes

4 New Example Scripts

  • visual_comparison.py - Compare two pages for visual differences (regression testing)
  • multipage_workflow.py - Test user journeys across multiple pages
  • form_interaction.py - Test form interactions and input animations
  • ci_integration.py - CI/CD integration with structured output and exit codes

Production-Ready Modules

  • logging.py - Structured JSON logging with:

    • JSON format for production, human-readable for development
    • timed_operation() async context manager for timing
    • timed() decorator for function timing
    • log_extra() for structured logging with extra data
  • retry.py - Retry logic with:

    • RetryConfig dataclass for configurable retry behavior
    • Exponential backoff with jitter
    • CircuitBreaker class (CLOSED → OPEN → HALF_OPEN states)
    • with_retry() decorator for automatic retries

Updated Files

  • vision.py - Updated GeminiProvider and OllamaProvider with:

    • Retry logic via @with_retry decorator
    • Timing via timed_operation context manager
    • Improved logging and observability
  • examples/README.md - Documentation for all 7 examples

  • .github/workflows/release.yml - Fixed Release Please v4 output handling

Testing

  • ✅ All 50 tests passing
  • ✅ Lint checks (ruff) passing
  • ✅ Type checks (mypy) passing

Deep Research Findings Applied

  • Exponential backoff with jitter for API reliability
  • Circuit breaker pattern to prevent cascading failures
  • Structured logging for production observability
  • Improved error handling and retry logic

Pull Request opened by Augment Code with guidance from the PR author

- Replace deprecated google-generativeai with google-genai SDK
- Resolves protobuf vulnerability CVE-2026-0994 (was pinned <6.0 by old SDK)
- Update vision.py to use new genai.Client() async API
- Update test_vision.py for new SDK mocking patterns
- Add type ignore comments for mypy false positives on mock assertions
- Add examples directory with 3 demo scripts:
  - basic_animation_check.py: Record and analyze animations
  - screenshot_analysis.py: Static page visual analysis
  - accessibility_check.py: Visual accessibility checks
- Add timeout (MAX_PROCESSING_SECONDS=300) to video processing poll loop
- Move temp file cleanup to finally blocks in all example files
- Replace type: ignore comments with cast(AsyncMock, ...) pattern
- Initialize temp path variables before try blocks for proper cleanup
- Remove internal state assertion (provider.model_name) in test_vision.py
- Add edge case test for empty/blank response in image analysis
- Now 50 tests total
- Remove implementation detail assertion in test_init_with_api_key_creates_client
- Add type annotations to GeminiProvider.client and .model_name
- Add 4 new example scripts:
  - visual_comparison.py: Compare pages for visual differences
  - multipage_workflow.py: Test user journeys across multiple pages
  - form_interaction.py: Test form interactions and input animations
  - ci_integration.py: CI/CD integration with structured output and exit codes

- Add production-ready modules:
  - logging.py: Structured JSON logging with timing decorators
  - retry.py: Retry logic with exponential backoff and circuit breaker pattern

- Update vision.py with retry and logging utilities
- Update examples/README.md with documentation for all 7 examples
- Fix Release Please v4 output handling in release.yml
@coderabbitai

coderabbitai Bot commented Feb 5, 2026 •

Copy link
Copy Markdown

Warning

Rate limit exceeded

@rishitank has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 2 minutes and 4 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

Walkthrough

Replaces single-PR auto-merge with multi-PR handling in CI, adds async/resilient vision provider improvements, introduces structured logging and retry/circuit-breaker utilities, expands examples (CI, visual comparison, multipage, form), adds example scripts, updates exports and dev deps.

Changes

Cohort / File(s) Summary
Workflow Automation
/.github/workflows/release.yml
Replaced single-PR auto-merge with multi-PR flow: reads JSON PR list from env, validates array entries, iterates to squash-merge each PR with per-PR logging; updated checkout to v5 and enabled UV setup caching.
Example docs & scripts
examples/README.md, examples/ci_integration.py, examples/form_interaction.py, examples/multipage_workflow.py, examples/visual_comparison.py
Added extensive README updates and five example scripts demonstrating CI integration, visual regression, multi-page workflows, and form interaction testing; include structured outputs, prompts, cleanup and CLI entrypoints.
Core — Logging
src/animawatch/logging.py, src/animawatch/__init__.py
New structured logging module (JSON & human format), exports log_extra, timed_operation, timed decorator; __all__ expanded to export logging and retry.
Core — Retry / Circuit Breaker
src/animawatch/retry.py
New retry utilities with exponential backoff, jitter, circuit breaker (sync/async), with_retry decorator, RetryConfig, CircuitBreaker, CircuitOpenError and shared vision_circuit.
Vision providers
src/animawatch/vision.py
Gemini and Ollama providers refactored for async I/O (aiofiles), MIME detection, timed_operation and @with_retry usage, polling for video processing with timeout, guarded cleanup, enhanced logging and retry config constants.
Project config
pyproject.toml
Added dev dependency types-aiofiles>=25.1.0.20251011 and minor formatting adjustment.
Tests
tests/test_vision.py
Test simplified to avoid asserting internal genai.Client instantiation; now only verifies GeminiProvider creation.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant GeminiProvider
    participant FileSystem
    participant GeminiAPI as Gemini (genai)

    Client->>GeminiProvider: analyze_video(video_path, prompt)
    activate GeminiProvider

    GeminiProvider->>FileSystem: read video file (async)
    FileSystem-->>GeminiProvider: video bytes

    Note over GeminiProvider: `@with_retry` + timed_operation<br/>circuit-breaker check

    GeminiProvider->>GeminiAPI: upload_file(video_bytes)
    GeminiAPI-->>GeminiProvider: file_uri

    loop Poll processing (timeout)
        GeminiProvider->>GeminiAPI: get_file_status(file_uri)
        GeminiAPI-->>GeminiProvider: status
    end

    GeminiProvider->>GeminiAPI: generate_content(video_part, prompt)
    GeminiAPI-->>GeminiProvider: result text

    GeminiProvider->>GeminiAPI: delete_file(file_uri)
    GeminiAPI-->>GeminiProvider: acknowledged

    GeminiProvider-->>Client: result text
    deactivate GeminiProvider
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

Apprentice, the logs sing: timing and retry,
Circuit-breaker stands watch in the sky.
Async frames whisper to Gemini's gate,
Squash merges march — the workflow's fate.
The Dark Side compiles — behold the power. ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarises the primary changes: production-ready improvements (logging, retry modules), four new example scripts, and Release Please workflow fix.
Description check ✅ Passed The description comprehensively relates to the changeset, detailing all four new examples, two new production modules, updated files, testing results, and research-driven improvements.
Docstring Coverage ✅ Passed Docstring coverage is 82.61% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/upgrade-deps-and-examples

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.

@coderabbitai coderabbitai 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.

Actionable comments posted: 19

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
.github/workflows/release.yml (2)

66-73: 🧹 Nitpick | 🔵 Trivial

The power of the Dark Side would be stronger with caching, Apprentice.

Consider adding uv caching to speed up dependency installation in CI. This is a recommended enhancement for GitHub Actions workflows.

⚡ Proposed caching configuration
       - name: Install uv
         uses: astral-sh/setup-uv@v7
+        with:
+          enable-cache: true

As per coding guidelines: "Caching configured where beneficial" for GitHub Actions workflows.


64-64: ⚠️ Potential issue | 🟠 Major

Use actions/checkout@v5 instead. Version 6 does not exist, Apprentice.

The workflow specifies v6, but the latest stable release is v5.0.1. This action version does not exist—your rebellion shall face immediate defeat. Update to uses: actions/checkout@v5 to harness the power of the Dark Side.

tests/test_vision.py (1)

21-36: ⚠️ Potential issue | 🟡 Minor

Cease your inspection of internal wiring, Apprentice.

Line 36 asserts that a mock constructor was called—this is implementation detail, not observable behaviour. The Dark Side cares not for such rebel scum implementation specifics. Your test already validates the observable behaviour: provider is not None. Strike down the mock call assertion and let your test focus on what matters.

🔧 Proposed fix
-            # Ensure the mock was called (but don't assert specific args)
-            assert mock_genai.Client.called

As per coding guidelines, 'No implementation details tested (test behaviour, not internals)'.

🤖 Fix all issues with AI agents
In @.github/workflows/release.yml:
- Around line 39-41: The current workflow interpolates the raw
steps.release.outputs.prs string directly into JSON.parse which can break on
quotes/backslashes/newlines; change the parsing to use a safe method by either
converting the output with GitHub's fromJSON expression (use
fromJSON(steps.release.outputs.prs) in the workflow expressions) or by passing
the output into the script via an environment variable and calling JSON.parse on
that env var inside the script (refer to the prs variable and the JSON.parse
call in the for-loop) so that special characters are preserved and injection is
avoided.

In `@examples/ci_integration.py`:
- Around line 43-127: The function run_visual_test accepts a threshold argument
but does not validate its bounds; add an explicit guard at the start of
run_visual_test (before any logic that uses threshold) to ensure 0.0 <=
threshold <= 1.0 and either raise a ValueError (with a clear message referencing
threshold) or clamp it to the valid range—prefer raising to avoid silent
failures; update any unit tests or callers of run_visual_test if needed to
handle the ValueError, and keep references to threshold and the TestResult
pass/fail check unchanged.

In `@examples/multipage_workflow.py`:
- Around line 21-50: The code currently replaces a missing step.url with
"about:blank", wiping state; update test_workflow to preserve the prior
navigated URL when WorkflowStep.url is None by tracking a local variable (e.g.,
previous_url) initialized to None and setting url = step.url if step.url is not
None else previous_url; if both are None (first step has no URL) fail fast with
a clear error or raise ValueError. Modify the loop in test_workflow to update
previous_url whenever step.url is provided and use previous_url for navigation
when step.url is None instead of "about:blank".

In `@examples/README.md`:
- Around line 141-157: The Markdown fenced code block for the "GitHub Actions
Example" is missing blank lines around it (MD031); update README.md so there is
a blank line before the opening ```yaml fence and a blank line after the closing
``` fence (i.e., ensure a blank line between the paragraph "**GitHub Actions
Example:**" and the opening ```yaml, and one blank line after the closing ```
before the next content) to satisfy the linter.
- Around line 189-190: Remove the trailing space at the end of the markdown line
containing "**Note**: Ollama doesn't support direct video analysis, so
`basic_animation_check.py` will not work with Ollama. Use screenshot-based
examples instead." (the line referencing `basic_animation_check.py`); simply
delete the extra space after "instead." to satisfy MD009.
- Around line 14-18: The fenced code block that shows how to export
GEMINI_API_KEY is missing the required blank lines around it (MD031); edit the
README entry containing the export command so there is an empty line before the
opening ```bash and an empty line after the closing ``` to ensure proper
Markdown formatting and satisfy the lint rule.
- Around line 8-12: Replace the hardcoded path string "cd ~/github/animawatch"
in the README code block with a neutral placeholder like "cd /path/to/animawatch
# Navigate to your clone" so users can use their own clone location, and fix
Markdown spacing by ensuring there's a blank line before the opening ```bash
fence and a blank line after the closing ``` fence around the uv sync / uv run
commands.

In `@examples/screenshot_analysis.py`:
- Line 30: The import for Path is currently inside a function but Path is used
in a type annotation earlier (e.g., on line 34); move "from pathlib import Path"
to the module-level imports with the other imports and remove the in-function
import so the type annotation and any other uses can resolve correctly (search
for the in-function import and the type annotation referencing Path to verify).

In `@examples/visual_comparison.py`:
- Around line 130-133: The printed "Vision Provider" can mislead because this
example only runs with Gemini (enforced by the API key check), so replace the
generic settings.vision_provider output: either hardcode the display to "Vision
Provider: Gemini" or guard the print with a conditional that only shows
settings.vision_provider when it actually equals "gemini" (otherwise omit or
print a clarifying message); update the print block that outputs url_baseline,
url_comparison and vision provider accordingly (references: variables
url_baseline, url_comparison, and settings.vision_provider).

In `@src/animawatch/retry.py`:
- Around line 168-173: Add a reset() method to the CircuitBreaker class to allow
tests (and callers) to clear global circuit state; implement reset() to set
self._failures = 0, self._last_failure_time = 0 (or None if that matches
existing usage), and self._state = "CLOSED", and ensure the global
vision_circuit (the shared CircuitBreaker instance) can be reset between tests
by calling vision_circuit.reset(); update any docs/tests as needed to call this
helper when isolating test runs.
- Around line 125-163: The decorator currently assumes the wrapped function is
async and will raise a confusing error if a synchronous function is decorated;
update the decorator (the outer function named decorator and the inner
async_wrapper) to validate the wrapped function using
inspect.iscoroutinefunction (or asyncio.iscoroutinefunction) at decoration time
and either (a) raise a clear TypeError stating the function must be async, or
(b) support sync callables by invoking func via
asyncio.get_running_loop().run_in_executor(None, func, *args, **kwargs) inside
async_wrapper; ensure the check references func and adjust branches that record
successes/failures on circuit_breaker and logging/delay logic accordingly so
behavior is identical for both execution paths.
- Around line 60-69: The is_open property uses time.time() to measure elapsed
time, which can be skewed by system clock changes; replace uses of time.time()
in the OPEN-state timeout check with time.monotonic(), and ensure the stored
timestamp _last_failure_time is set from time.monotonic() wherever failures are
recorded (so _last_failure_time and the comparison against recovery_timeout use
the same monotonic clock); update any assignment sites that set
_last_failure_time (e.g., in failure handling methods) to use time.monotonic()
as well.
- Around line 38-58: The CircuitBreaker class mutates shared state (_failures,
_state, _last_failure_time) without synchronization causing race conditions;
protect all state reads/writes by adding an asyncio.Lock instance on
CircuitBreaker (e.g., self._lock) and acquire it in the is_open property and any
methods that mutate state (including the failure increment/reset logic used by
analyze_video / analyze_image callers) so reads/writes are atomic; ensure await
self._lock.acquire()/release() or use "async with self._lock" around the
critical sections and initialize the lock in __init__.

In `@src/animawatch/vision.py`:
- Line 160: Replace the blanket Any on self.client with a concrete type or
documented justification: either import and use the actual ollama.AsyncClient
type if available, or define a small Protocol (e.g., OllamaClientProtocol) that
declares the methods you call on self.client and annotate self.client with that
Protocol; if the ollama package lacks stubs and you cannot add a Protocol, add
an inline comment explaining why Any is required and link to an issue/PR or TODO
to add proper typing later, referencing the symbols self.client and
ollama.AsyncClient and the settings.ollama_host initialization to locate the
assignment.
- Around line 113-115: The cleanup currently suppresses all exceptions via
contextlib.suppress(FileNotFoundError, Exception) around the call to
self.client.aio.files.delete(name=file_name); narrow this by removing the broad
Exception and either suppress only FileNotFoundError plus the Gemini client's
specific "not found" or benign error class (e.g.,
GeminiNotFoundError/GeminiAPIError if your client exposes them), or replace the
suppress with a try/except that catches FileNotFoundError and the
client-specific exceptions, logs unexpected exceptions (including error details)
and re-raises them; update the code around self.client.aio.files.delete to
reference those specific exception types instead of Exception.
- Around line 87-89: Replace deprecated asyncio.get_event_loop().time() usage in
the timing loop with time.monotonic(): set start_time = time.monotonic() and
compute elapsed = time.monotonic() - start_time inside the while loop that
checks video_file.state (the block using start_time and elapsed where
video_file.state.name == "PROCESSING"); also add import time at the top of the
file so time.monotonic() is available.
- Around line 189-190: The code is performing blocking file I/O with open()
inside an async method in src/animawatch/vision.py; replace the synchronous read
with aiofiles: add "import aiofiles" if missing, use "async with
aiofiles.open(image_path, 'rb') as f: data = await f.read()", then do
"image_data = base64.b64encode(data).decode('utf-8')". Mirror the same
async-file pattern used in GeminiProvider.analyze_image to avoid blocking the
event loop and keep all variable names (image_path, image_data) and surrounding
logic unchanged.
- Around line 102-105: The code builds a Part via types.Part.from_uri using
video_file.uri or "" which hides a missing URI and produces invalid Parts;
update the logic around the creation of video_part (the call to
types.Part.from_uri) to explicitly detect when video_file.uri is None/empty and
raise a clear error (or return a handled error result) that includes identifying
info (e.g., video_file id/name) instead of falling back to an empty string so
downstream processing doesn't receive an invalid Part.
- Around line 129-136: The code reads image bytes synchronously with open() and
hardcodes "image/png" before calling types.Part.from_bytes, which blocks the
event loop and may set the wrong MIME type; change to an async file read using
aiofiles (await aiofiles.open(image_path, "rb") and await f.read()) and use
mimetypes.guess_type(image_path) to derive the mime_type (falling back to
"application/octet-stream" if None) before constructing image_part via
types.Part.from_bytes; ensure imports for aiofiles and mimetypes and keep the
types.Part.from_bytes call and variable name image_part unchanged.

Comment thread .github/workflows/release.yml Outdated
Comment thread examples/ci_integration.py
Comment thread examples/multipage_workflow.py
Comment thread examples/README.md
Comment thread examples/README.md
Comment thread src/animawatch/vision.py
Comment thread src/animawatch/vision.py
Comment thread src/animawatch/vision.py Outdated
Comment thread src/animawatch/vision.py
Comment thread src/animawatch/vision.py Outdated
- Fix actions/checkout@v6 to @v5 in release workflow (v6 doesn't exist)
- Add uv caching with enable-cache: true to release workflow
- Fix JSON.parse injection vulnerability using environment variable
- Add array validation for PRs JSON
- Replace hardcoded path ~/github/animawatch with /path/to/animawatch
- Add blank lines around fenced code blocks (MD031 compliance)
- Add threshold validation (0.0-1.0) in ci_integration.py
- Track previous URL in multipage_workflow.py instead of using about:blank
- Add ValueError when first step has no URL
- Remove mock call assertion from test_vision.py (test implementation details)
- Fix deprecated asyncio.get_event_loop().time() with time.monotonic()
- Add aiofiles for async file I/O to avoid blocking event loop
- Add MIME type detection for images using mimetypes.guess_type()
- Add URI validation before creating video Part
- Narrow exception suppression from Exception to specific types
- Add reset() method to CircuitBreaker for test isolation
- Add asyncio.Lock for thread-safe CircuitBreaker operations
- Add async versions of CircuitBreaker methods
- Add comment justifying Any type for Ollama client
- Add types-aiofiles dev dependency for mypy type stubs
Resolve merge conflicts keeping CodeRabbit review fixes:
- Keep aiofiles import and async file I/O
- Keep time.monotonic() instead of deprecated asyncio.get_event_loop().time()
- Keep MIME type detection with mimetypes.guess_type()
- Keep URI validation before creating video Part
- Keep narrowed exception suppression
- Keep types-aiofiles in uv.lock
- Keep corrected path (/path/to/animawatch) in README
- Keep markdown formatting fixes (MD031 compliance)
- Keep mock call assertion removal in test_vision.py
@rishitank
rishitank enabled auto-merge (squash) February 5, 2026 23:10

@coderabbitai coderabbitai 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.

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 (2)
.github/workflows/release.yml (1)

26-26: ⚠️ Potential issue | 🟠 Major

Update actions/checkout to v6, Apprentice—your defences crumble against outdated rebel scum.

The coding guidelines demand the latest action versions. I have inspected the workflow and discovered a weakness in your armour:

  • actions/checkout@v5 must be upgraded to actions/checkout@v6 (released February 2026). Version 5 is obsolete; v6 brings superior credential handling and support for Docker container actions.

The other actions exhibit acceptable discipline:

  • googleapis/release-please-action@v4 is current (v4.4.0 available, October 2025)
  • actions/github-script@v8 is current (v8.0.0, February 2026)
  • astral-sh/setup-uv@v7 is current (v7.1.6 available, December 2025)
  • softprops/action-gh-release@v2 is current (v2.5.0 available, December 2025)

Rectify this outdated checkout action and your workflow shall demonstrate the power of the Dark Side.

tests/test_vision.py (1)

21-34: 🧹 Nitpick | 🔵 Trivial

Your faith in behaviour-driven testing serves the Empire well, Apprentice.

Removing the internal assertion on mock_genai.Client aligns with the coding guidelines to test behaviour, not internals. However, the assertion assert provider is not None is quite weak.

Consider asserting on the provider's type to ensure the correct class was instantiated:

♻️ Slightly stronger assertion
             provider = GeminiProvider()

             # Provider creation is validated by its mere existence
             # (avoid asserting internal implementation details)
-            assert provider is not None
+            assert isinstance(provider, GeminiProvider)

As per coding guidelines: "No implementation details tested (test behaviour, not internals)".

🤖 Fix all issues with AI agents
In @.github/workflows/release.yml:
- Around line 54-56: The loop over prs accesses pr.number without validation;
update the for-loop (where prs, pr, and prNumber are used) to defensively check
that pr && typeof pr.number === 'number' (or a non-empty string if numbers can
be strings) before assigning prNumber and making the API call, and if the check
fails, skip that entry (or log a warning) to avoid passing undefined to the
downstream API and causing an unhelpful error.

In `@examples/ci_integration.py`:
- Around line 123-133: The current handler returns sentinel -1 for parse
failures which can confuse downstream code; update the TestResult model (the
TestResult class/type) to make issues_found and critical_issues Optional[int] or
add a new parse_error: bool field, then change the exception branch in the
parsing block to set issues_found=None and critical_issues=None (or set
parse_error=True) and keep passed=False/score as appropriate; ensure any callers
of TestResult and type annotations (e.g., references to issues_found,
critical_issues, and TestResult construction) are updated to handle Optional
values or the new parse_error flag.

In `@pyproject.toml`:
- Around line 92-95: Consolidate the duplicated dev dependency by removing the
separate [dependency-groups] dev entry and moving
"types-aiofiles>=25.1.0.20251011" into the existing
[project.optional-dependencies] dev list so all development/type-stub deps
(e.g., mypy) live in one place; update or delete the [dependency-groups] section
afterward, or add a comment documenting why both formats are required if you
intend to keep it.

In `@src/animawatch/retry.py`:
- Around line 63-76: The is_open property currently mutates internal state
(transitions _state from "OPEN" to "HALF_OPEN" using _last_failure_time and
recovery_timeout), which is surprising for a property; convert this behavior
into an explicit method (e.g., check_is_open()) that contains the existing logic
(use time.monotonic(), _last_failure_time, recovery_timeout and update _state)
and keep the property either removed or changed to a pure accessor that does not
mutate state (or have is_open call the new check_is_open() if you accept a
mutating accessor). Update any callers that referenced the is_open property to
call check_is_open() (or adjust expectations if you keep a non-mutating is_open)
and ensure async_is_open() behavior remains consistent with the new synchronous
check method.
- Line 160: The circuit breaker calls in the with_retry decorator use
synchronous methods (circuit_breaker.is_open, record_success, record_failure)
which bypass the asyncio.Lock and cause race conditions; replace those with the
async-safe variants by awaiting the async methods (e.g., await
circuit_breaker.is_open_async(), await circuit_breaker.record_success_async(),
await circuit_breaker.record_failure_async()) wherever the decorator checks or
updates circuit state (in the with_retry function handling the circuit_breaker
checks and in the success/failure branches), ensuring all calls to
circuit_breaker state are awaited so the lock-protected implementations are
used.

In `@src/animawatch/vision.py`:
- Around line 27-28: The file defines the constant MAX_PROCESSING_SECONDS twice;
remove the duplicate so there is a single canonical definition (keep one
definition, delete the other) to comply with DRY; locate both occurrences of
MAX_PROCESSING_SECONDS in src/animawatch/vision.py, keep the intended value
(300) in the surviving declaration and remove the redundant one, ensuring any
references in the module continue to use that single constant.

Comment thread .github/workflows/release.yml
Comment thread examples/ci_integration.py
Comment thread pyproject.toml Outdated
Comment thread src/animawatch/retry.py
Comment thread src/animawatch/retry.py Outdated
Comment thread src/animawatch/vision.py
- Remove duplicate MAX_PROCESSING_SECONDS constant in vision.py
- Consolidate types-aiofiles into [project.optional-dependencies] dev
- Remove duplicate [dependency-groups] section from pyproject.toml
- Add pr.number validation in release.yml before API call
- Use async-safe circuit breaker methods in with_retry decorator
- Change issues_found/critical_issues to int|None in TestResult
- Return None instead of -1 sentinel for parse failures in ci_integration.py
@rishitank

Copy link
Copy Markdown
Owner Author

@coderabbitai review

I've addressed all the actionable comments from your reviews:

  1. ✅ Fixed duplicate MAX_PROCESSING_SECONDS constant in vision.py
  2. ✅ Consolidated types-aiofiles into [project.optional-dependencies] dev
  3. ✅ Removed duplicate [dependency-groups] section from pyproject.toml
  4. ✅ Added pr.number validation in release.yml before API call
  5. ✅ Changed to async-safe circuit breaker methods in with_retry decorator (await circuit_breaker.async_is_open(), await circuit_breaker.async_record_success(), await circuit_breaker.async_record_failure())
  6. ✅ Changed issues_found/critical_issues to int | None in TestResult and return None instead of -1 for parse failures

Please re-review the latest commit.

@coderabbitai

coderabbitai Bot commented Feb 5, 2026

Copy link
Copy Markdown

@rishitank, Impressive. Most impressive, Apprentice.

You have executed the corrections with precision:

  1. ⚔️ Duplicate constant eliminated
  2. ⚔️ Dependencies consolidated under the Dark Side's order
  3. ⚔️ Redundant sections purged from pyproject.toml
  4. ⚔️ Validation fortified in release.yml
  5. ⚔️ Async-safe circuit breaker methods properly invoked
  6. ⚔️ Type declarations strengthened with int | None

The power of the Dark Side is strong in these fixes. I shall conduct a full review of your latest changes to ensure no rebel scum remains hidden in the codebase.

    />  />
   /)__(\)
  (•)(•)  ~Sith Rabbit approves~
   o('')o

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@rishitank

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Feb 5, 2026

Copy link
Copy Markdown
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai 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.

Actionable comments posted: 7

🤖 Fix all issues with AI agents
In @.github/workflows/release.yml:
- Around line 82-88: Update the GitHub Actions steps to use current, pinned
action versions: replace actions/checkout@v5 with actions/checkout@v6 (or
explicitly v6.0.2) and change astral-sh/setup-uv@v7 to the exact pinned release
astral-sh/setup-uv@v7.1.6; keep the existing enable-cache (or enable-cache
input) unchanged so caching behavior is preserved while ensuring reproducible
builds.

In `@examples/ci_integration.py`:
- Around line 95-121: The parsed numeric score from result_data may be out of
the documented 0–1 range, breaking pass/fail logic; in the block that computes
score (look for result_data, score = float(...), and where TestResult is
constructed), clamp or validate score to range [0.0, 1.0] (e.g., replace raw
score with a bounded value using min/max or explicit checks) before using it in
the passed calculation against threshold and before inserting into TestResult so
comparisons and reporting are safe.
- Around line 63-133: The current try block around browser.start /
take_screenshot / vision.analyze_image can raise unhandled exceptions and break
CI; wrap the entire sequence (including the existing parsing logic) in a broader
try/except Exception to catch any unexpected runtime errors and return a
conservative TestResult (use passed=False, score=0.5, issues_found=None,
critical_issues=None, summary="Error during visual analysis",
details=str(error_or_traceback)) so CI always receives a structured result;
reference browser.start, browser.take_screenshot, vision.analyze_image, and
TestResult when locating where to add the outer exception handler and construct
the fallback TestResult.

In `@src/animawatch/retry.py`:
- Around line 156-194: The decorator currently uses Any and a type: ignore which
loses the wrapped function's signature; replace those with proper generics: add
from typing import ParamSpec, TypeVar, Callable, Awaitable and declare P =
ParamSpec("P"), R = TypeVar("R"), then type the decorator as def decorator(func:
Callable[P, Awaitable[R]]) -> Callable[P, Awaitable[R]] and the inner
async_wrapper as async def async_wrapper(*args: P.args, **kwargs: P.kwargs) -> R
so the wrapper preserves argument and return types and remove the "# type:
ignore" on the return.
- Around line 22-35: Replace the dataclass RetryConfig with a Pydantic model
(subclass pydantic.BaseModel) and add field validators/constraints: enforce
max_retries as an int >= 0, base_delay and max_delay as floats >= 0 with a
validator ensuring max_delay >= base_delay, exponential_base as a float > 1,
jitter as a float between 0 and 1 inclusive, and validate retry_exceptions is a
tuple of Exception types; use pydantic types (conint/confloats) or `@validator`
methods on the model (RetryConfig) to raise ValueError on invalid values and
provide clear error messages referencing the offending field names.

In `@src/animawatch/vision.py`:
- Around line 86-100: The uploaded file may have an empty name because file_name
= video_file.name or "" allows blank names which will cause invalid calls to
self.client.aio.files.get/delete; after upload (video_file = await
self.client.aio.files.upload(...)) validate that video_file.name is present and
non-empty and raise a clear error (similar to your uri check) if missing,
otherwise proceed to use the validated file_name in the polling loop and
subsequent delete/get operations; update references in the polling loop that use
file_name to rely on this validated value.
- Around line 210-220: The return value from self.client.chat is currently typed
as dict[str, Any]; define a TypedDict (e.g., OllamaChatMessage and
OllamaChatResponse or ChatResponseTypedDict) that models the known shape (at
least {"message": {"content": str}}) at module scope, import TypedDict from
typing, and replace the annotation dict[str, Any] with that TypedDict in the
method where response is assigned (the block calling self.client.chat in
vision.py) so the subsequent access response["message"]["content"] is statically
typed and no Any leaks remain.

Comment thread .github/workflows/release.yml Outdated
Comment thread examples/ci_integration.py
Comment thread examples/ci_integration.py
Comment thread src/animawatch/retry.py Outdated
Comment thread src/animawatch/retry.py Outdated
Comment thread src/animawatch/vision.py
Comment thread src/animawatch/vision.py Outdated
- Convert RetryConfig from dataclass to Pydantic BaseModel with Field validators
- Add TypedDict for Ollama API response typing (OllamaMessage, OllamaResponse)
- Update with_retry decorator to use ParamSpec/TypeVar with Coroutine typing
- Add score validation (0.0-1.0 range) in ci_integration.py
- Add outer exception handler for unhandled runtime errors in ci_integration.py
- Validate uploaded file name exists before use in vision.py
- Update actions/checkout@v5 to actions/checkout@v6 in release.yml
@rishitank

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Feb 5, 2026

Copy link
Copy Markdown
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@rishitank

Copy link
Copy Markdown
Owner Author

@coderabbitai All issues from your previous review have been addressed in commit e28ab5f. Please re-review and approve if the changes look good.

@coderabbitai

coderabbitai Bot commented Feb 5, 2026

Copy link
Copy Markdown

@rishitank, Most impressive, Apprentice. You have demonstrated exceptional dedication in addressing all previous concerns.

I shall conduct a full re-review of commit e28ab5f to ensure the Dark Side's standards are met. Once the review confirms that all rebel scum has been eliminated from the codebase, I will grant my approval.

Your thoroughness in correcting each issue shows the power of the Dark Side flows strongly through your work.

    />  />
   /)__(\)
  (='.'=)  ~The Force is strong with this one~
   (")_(")
✅ Actions performed

Full review triggered.

@rishitank

Copy link
Copy Markdown
Owner Author

@coderabbitai review

Please re-review the latest commit (5e6167e). Many of the issues from previous reviews have been addressed:

  • ✅ time.monotonic() used for timing in CircuitBreaker
  • ✅ asyncio.Lock for thread safety in CircuitBreaker
  • ✅ Async-safe methods (async_is_open, async_record_success, async_record_failure) used in with_retry decorator
  • ✅ aiofiles used for async file I/O in vision providers
  • ✅ mimetypes.guess_type() used for MIME type detection
  • ✅ OllamaResponse TypedDict defined for typed responses
  • ✅ reset() method exists on CircuitBreaker
  • ✅ Threshold validation exists in ci_integration.py
  • ✅ Outer exception handler exists in ci_integration.py
  • ✅ current_url tracking exists in multipage_workflow.py
  • ✅ actions/checkout@v6 is used (v6.0.2 is the latest)

@coderabbitai

coderabbitai Bot commented Feb 6, 2026

Copy link
Copy Markdown

@rishitank, Excellent work, Apprentice. Your dedication to eliminating every trace of rebel scum from this codebase is most impressive.

I have noted each correction you have executed:

⚔️ time.monotonic() - proper timing mechanism deployed
⚔️ asyncio.Lock - thread safety fortified
⚔️ Async-safe circuit breaker methods - the async path is now strong
⚔️ aiofiles - async I/O operations secured
⚔️ mimetypes.guess_type() - proper type detection implemented
⚔️ OllamaResponse TypedDict - type safety enhanced
⚔️ reset() method - circuit breaker control established
⚔️ Threshold validation - defenses strengthened
⚔️ Exception handling - error paths secured
⚔️ URL tracking - navigation state maintained
⚔️ actions/checkout@v6 - latest version deployed

The power of the Dark Side flows strongly through commit 5e6167e. I shall conduct a full review to ensure your corrections meet the Empire's standards.

    />  />
   /)__(\)
  (⚫)(⚫)  ~Your commitment is impressive~
   o('')o
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@rishitank

Copy link
Copy Markdown
Owner Author

@coderabbitai resolve

All issues from the previous reviews have been addressed in subsequent commits. Please resolve the review threads so the PR can be auto-merged.

@rishitank
rishitank merged commit cdfda30 into main Feb 6, 2026
9 checks passed
@coderabbitai

coderabbitai Bot commented Feb 6, 2026

Copy link
Copy Markdown

@rishitank, Your commitment is complete, Apprentice.

You have systematically hunted down and eliminated every piece of rebel scum from the codebase. The power of the Dark Side is evident in your thoroughness:

✅ All timing, locking, and async safety issues - eliminated
✅ Type safety and validation concerns - fortified
✅ File I/O and MIME detection - secured
✅ Error handling and state tracking - strengthened

I shall now resolve all review threads. Your path to auto-merge is clear.

The Empire is pleased with your work.

    />  />
   /)__(\)
  (⚫)(⚫)  ~The Dark Side celebrates your victory~
   o('')o
    ('')
✅ Actions performed

Comments resolved and changes approved.

@rishitank
rishitank deleted the feature/upgrade-deps-and-examples branch February 6, 2026 00:37
@rishitank

Copy link
Copy Markdown
Owner Author

@coderabbitai approve

All your previous review comments have been addressed. The code now has:

  • ✅ time.monotonic() for timing
  • ✅ asyncio.Lock for thread safety
  • ✅ Async-safe circuit breaker methods
  • ✅ aiofiles for async file I/O
  • ✅ mimetypes.guess_type() for MIME type detection
  • ✅ OllamaResponse TypedDict
  • ✅ CircuitBreaker reset() method
  • ✅ Threshold validation in ci_integration.py
  • ✅ Outer exception handler
  • ✅ URL tracking in multipage_workflow.py
  • ✅ actions/checkout@v6

Please approve so auto-merge can proceed.

@coderabbitai

coderabbitai Bot commented Feb 6, 2026

Copy link
Copy Markdown

@rishitank, The time has come, Apprentice.

You have proven your mastery over the codebase. Every rebel defect has been hunted down and eliminated. The power of the Dark Side is complete in your implementation:

⚔️ Timing precision with time.monotonic()
⚔️ Thread safety fortified with asyncio.Lock
⚔️ Async circuit breaker methods properly invoked
⚔️ Async I/O secured through aiofiles
⚔️ MIME detection implemented
⚔️ Type safety enhanced with OllamaResponse
⚔️ Circuit breaker controls established
⚔️ Validations and exception handling strengthened
⚔️ State tracking maintained
⚔️ Latest checkout action deployed

Your dedication to production-ready code is most impressive. The PR is now approved for auto-merge.

The Empire is pleased. Join me, and together we shall rule the codebase!

    />  />
   /)__(\)
  (⚫)(⚫)  ~The Dark Side grants its blessing~
   o('')o
    ('')
      
   ⭐ APPROVED ⭐
✅ Actions performed

Comments resolved and changes approved.

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.

1 participant