perf(cloud-ai): run AWS Rekognition boto3 calls off the event loop - #1205
Conversation
boto3 is a synchronous SDK. Every Rekognition call in AWSRekognition was issued directly inside an `async def`, so each one blocked the event loop for a full network round-trip. `_wait_for_job_completion` is the worst case: it polls every 5s for up to 600s, so a single video analysis could stall the loop up to 120 times. All 14 boto3 calls now dispatch via `await asyncio.to_thread(...)`, and the local-image read in `_prepare_image_input` goes through a new module-level `_read_file_bytes` helper on the same path. - 89 pre-existing tests pass with zero edits - 6 new heartbeat tests (`TestRekognitionDoesNotBlockEventLoop`); 5 of the 6 discriminate, proven by reverting both dimensions simultaneously (5 targeted failures / 90 passed) - ruff: exact parity with origin/main (8 pre-existing findings, 0 added) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are limited based on label configuration. 🏷️ Required labels (at least one) (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository YAML (base), Repository UI (inherited), Organization UI (inherited) Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughSummary by CodeRabbit
WalkthroughAWS Rekognition’s blocking boto3 calls now run through ChangesAWS Rekognition asynchronous I/O
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 5 | ❌ 2❌ Failed checks (2 inconclusive)
✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Warning Review ran into problems🔥 ProblemsThese MCP integrations need to be re-authenticated in the Integrations settings: Sentry Linked repositories: Public OSS repositories can only analyze public repositories installed in this organization. No linked repositories were analyzed; skipped Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review Adversarial challenges — please try to break these specifically:
Context: reviewers have caught genuine bugs in my previous PRs here (#1186, #1188, #1190, #1194) — including two regressions I introduced myself. Please be equally sceptical. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
provider='src/youtube_extension/integrations/cloud_ai/providers/aws_rekognition.py'
echo '=== Changed files ==='
git diff --name-only origin/main...HEAD
echo '=== Provider outline ==='
ast-grep outline "$provider" --items all --type function,class --view expanded
echo '=== Integrator lifecycle references ==='
rg -n -C 8 --type py 'await\s+provider\.cleanup\s*\(|async\s+def\s+.*cleanup|provider\.cleanup' src
echo '=== Provider call sites and cleanup ==='
rg -n -C 5 --type py \
'asyncio\.to_thread|_rekognition_client|def\s+_read_file_bytes|async\s+def\s+_prepare_image_input|async\s+def\s+_wait_for_job_completion|async\s+def\s+analyze_image|async\s+def\s+get_service_status|async\s+def\s+_test_connection|async\s+def\s+cleanup' \
"$provider"
echo '=== Tests mentioning provider or heartbeat ==='
fd -e py . | xargs rg -n -l -i 'AWSRekognition|heartbeat|_wait_for_job_completion|analyze_image|get_service_status|_test_connection' || true
echo '=== PR test diff ==='
git diff --unified=80 origin/main...HEAD -- \
"$provider" \
'*aws_rekognition*' \
'*rekognition*'Length of output: 85988 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== CloudAIIntegrator public analysis and cleanup methods ==='
sed -n '35,180p' src/youtube_extension/integrations/cloud_ai/integrator.py
echo '=== Integrator construction, cleanup, and analysis call sites ==='
rg -n -C 8 --type py \
'(CloudAIIntegrator\s*\(|cloud_ai_integrator|\.analyze_image\s*\(|\.analyze_video\s*\(|\.cleanup\s*\()' \
src/youtube_extension \
-g '!src/youtube_extension/integrations/cloud_ai/integrator.py'
echo '=== Relevant provider methods with complete control flow ==='
sed -n '60,250p' src/youtube_extension/integrations/cloud_ai/providers/aws_rekognition.py
sed -n '260,410p' src/youtube_extension/integrations/cloud_ai/providers/aws_rekognition.py
echo '=== Test module setup, fixtures, and async configuration ==='
sed -n '1,180p' tests/unit/test_aws_rekognition_provider.py
fd -HI 'pyproject.toml' 'pytest.ini' 'tox.ini' 'setup.cfg' . -d 3 -x sh -c \
'echo "=== $1 ==="; rg -n -C 4 "asyncio|pytest" "$1" || true' sh {}
echo '=== All changes to the target test module, with concise context ==='
git diff --unified=25 origin/main...HEAD -- tests/unit/test_aws_rekognition_provider.pyLength of output: 42949 🏁 Script executed: #!/bin/bash
set -u
echo '=== Locate Rekognition test files ==='
fd -HI -t f '.*aws.*rekognition.*\.py$' . || true
echo '=== Test module locations in the PR diff ==='
git diff --name-only origin/main...HEAD | nl -ba
echo '=== Pytest and asyncio configuration ==='
for file in $(fd -HI -t f 'pyproject.toml|pytest.ini|tox.ini|setup.cfg' . -d 4); do
echo "=== $file ==="
rg -n -C 5 'pytest|asyncio|testpaths|python_files' "$file" || true
done
echo '=== Cloud AI router: request and batch lifecycle ==='
sed -n '200,420p' src/youtube_extension/backend/cloud_ai_routes.py
echo '=== Cloud AI integrator construction and all provider map mutations ==='
rg -n -C 6 --type py \
'(self\.providers|CloudAIIntegrator\s*\(|get_cloud_ai_config\s*\()' \
src/youtube_extension/integrations/cloud_ai/integrator.py \
src/youtube_extension/backend/cloud_ai_routes.py
echo '=== Precise await-to-thread call count in the changed provider ==='
ast-grep run --pattern 'await asyncio.to_thread($$$ARGS)' \
--lang python \
src/youtube_extension/integrations/cloud_ai/providers/aws_rekognition.pyLength of output: 42681
You are interacting with an AI system. |
There was a problem hiding this comment.
Pull request overview
Offloads synchronous AWS Rekognition operations from the event loop.
Changes:
- Dispatches 14 boto3 calls through
asyncio.to_thread. - Offloads local image reads.
- Adds responsiveness and behavior tests.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
aws_rekognition.py |
Moves blocking AWS and file operations to worker threads. |
test_aws_rekognition_provider.py |
Adds event-loop responsiveness tests. |
…elapsed ticks The heartbeat form of this one test failed on CI. Unlike the four boto3 tests, which drive a controllable 0.12s mock, the local file read is a few microseconds of real work, so "did the loop tick while it ran" is a load-sensitive proxy rather than a property. Assert the property directly instead: record `threading.get_ident()` inside `_read_file_bytes` and require it to differ from the thread running the event loop. That is exactly what "dispatched off the loop" means, needs no sleeps, and cannot flake under runner contention. - 95 tests pass (89 pre-existing, unmodified, + 6 new) - Non-vacuity: calling `_read_file_bytes` directly instead of via `asyncio.to_thread` yields exactly 1 targeted failure / 94 passed - Suite runtime for the file drops to 0.85s (the 0.12s sleep is gone) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Agent Completion Truth Gate: NOT_APPLICABLEEvidence agrees. Machine-readable verdict{
"details": {},
"reasons": [],
"verdict": "not_applicable"
} |
|
@coderabbitai review New head Specific challenges for this round:
|
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
provider='src/youtube_extension/integrations/cloud_ai/providers/aws_rekognition.py'
echo '=== Changed files ==='
git diff --name-only origin/main...HEAD
echo '=== Provider outline ==='
ast-grep outline "$provider" --items all --type function,class --view expanded
echo '=== Provider: imports, cleanup, local-image preparation, and thread dispatch ==='
rg -n -C 6 --type py \
'import asyncio|import threading|def _read_file_bytes|async def cleanup|async def _prepare_image_input|asyncio\.to_thread|_rekognition_client|_s3_client' \
"$provider"
echo '=== Rekognition tests: outline and revised-test contexts ==='
fd -HI -t f '.*aws.*rekognition.*\.py$' . -x sh -c '
echo "=== $1 ==="
ast-grep outline "$1" --items all --type function,class --view expanded
rg -n -C 12 --type py \
"_read_file_bytes|threading\.get_ident|patch\.object|heartbeat|tick|sleep|to_thread|local_image" \
"$1"
' sh {}
echo '=== Exact PR diff for provider and relevant tests ==='
git diff --unified=50 origin/main...HEAD -- \
"$provider" \
'tests/unit/test_aws_rekognition_provider.py'
echo '=== Integrator cleanup lifecycle ==='
sed -n '45,110p' src/youtube_extension/integrations/cloud_ai/integrator.pyLength of output: 54768 🤖 web_search executed: `Official Python asyncio.to_thread documentation: does asyncio.to_thread run the callable in a separate thread, and can it run inline on the event-loop thread? Official boto3 documentation: are low-level boto3 clients thread-safe, and what does client close() do for in-flight requests or client cleanup?` Length of output: 12216
The revised local-image test is a correct replacement for the flaky heartbeat test. No changes requested. 🐇 ✅ Action performedReview finished.
|
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Snapshot WarningsEnsure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice. Scanned FilesNone |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/youtube_extension/integrations/cloud_ai/providers/aws_rekognition.py (1)
260-278: 🎯 Functional Correctness | 🔴 Critical | 🏗️ Heavy liftRemove the production stub in
_ensure_video_in_s3.Lines 260-278 return a guessed S3 key for every non-
s3://input. The code only logs that an upload would occur. It does not upload the video. Lines 294-320 then submit that nonexistent object to Rekognition, so HTTP and local video inputs fail in production.Reject non-S3 inputs until upload support exists, or implement the upload with verified bucket and key handling before starting the Rekognition jobs. Remove the demo fallback.
As per path instructions, flag placeholder or stub implementations as blocking issues.
Also applies to: 294-320
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/youtube_extension/integrations/cloud_ai/providers/aws_rekognition.py` around lines 260 - 278, Remove the non-S3 demo fallback from _ensure_video_in_s3: reject inputs that do not start with s3:// instead of constructing a guessed bucket/key and logging a placeholder upload. Preserve valid S3 URL parsing, and ensure the Rekognition job flow only proceeds with verified existing S3 objects until real upload support is implemented.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/youtube_extension/integrations/cloud_ai/providers/aws_rekognition.py`:
- Around line 34-37: Update analyze_image() and _read_file_bytes() to constrain
local image_url paths to the configured media root: resolve the candidate path,
reject absolute paths and traversal that resolve outside the root, and only then
pass the validated path to asyncio.to_thread. Preserve existing handling for S3
and HTTP URLs.
- Around line 113-115: Update initialize() to create the Rekognition and S3
clients with botocore.config.Config specifying explicit connect_timeout and
read_timeout values, ensuring requests invoked through asyncio.to_thread are
bounded without changing their existing behavior.
---
Outside diff comments:
In `@src/youtube_extension/integrations/cloud_ai/providers/aws_rekognition.py`:
- Around line 260-278: Remove the non-S3 demo fallback from _ensure_video_in_s3:
reject inputs that do not start with s3:// instead of constructing a guessed
bucket/key and logging a placeholder upload. Preserve valid S3 URL parsing, and
ensure the Rekognition job flow only proceeds with verified existing S3 objects
until real upload support is implemented.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Repository UI (inherited), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3ec183c6-3bc7-4682-8de3-f46b3c05ce14
⛔ Files ignored due to path filters (1)
tests/unit/test_aws_rekognition_provider.pyis excluded by!tests/**
📒 Files selected for processing (1)
src/youtube_extension/integrations/cloud_ai/providers/aws_rekognition.py
📜 Review details
⚠️ CI failures not shown inline (16)
GitHub Actions: Agent completion enforcement / 0_Agent completion enforcement.txt: perf(cloud-ai): run AWS Rekognition boto3 calls off the event loop
Conclusion: failure
##[group]Run actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3
with:
script: const fs = require('fs');
const pull = await github.rest.pulls.get({
owner: context.repo.owner,
repo: context.repo.repo,
pull_number: Number(process.env.PR)
});
let verdict = {
conclusion: 'failure',
reason: 'verifier_did_not_publish',
details: {}
};
try {
verdict = JSON.parse(fs.readFileSync(
'enforcement-verdict.json', 'utf8'
));
} catch (error) {
core.warning(error.message);
}
const conclusion = verdict.conclusion === 'success'
? 'success'
: 'failure';
const summary = JSON.stringify(verdict);
await github.rest.checks.create({
owner: context.repo.owner,
repo: context.repo.repo,
name: 'Agent completion enforcement',
head_sha: pull.data.head.sha,
status: 'completed',
conclusion,
output: {
title: conclusion === 'success'
? 'Trusted evidence verified'
: 'Trusted evidence blocked',
summary: summary.slice(0, 60000)
}
});
if (conclusion !== 'success') {
core.setFailed(verdict.reason || 'trusted evidence blocked');
}
github-***REDACTED_SECRET_ASSIGNMENT***
debug: false
user-agent: actions/github-script
result-encoding: json
retries: 0
retry-exempt-status-codes: 400,401,403,404,422
env:
PR: 1205
##[endgroup]
##[error]missing_trusted_publication
GitHub Actions: Agent completion enforcement / Agent completion enforcement: perf(cloud-ai): run AWS Rekognition boto3 calls off the event loop
Conclusion: failure
##[group]Run actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3
with:
script: const fs = require('fs');
const pull = await github.rest.pulls.get({
owner: context.repo.owner,
repo: context.repo.repo,
pull_number: Number(process.env.PR)
});
let verdict = {
conclusion: 'failure',
reason: 'verifier_did_not_publish',
details: {}
};
try {
verdict = JSON.parse(fs.readFileSync(
'enforcement-verdict.json', 'utf8'
));
} catch (error) {
core.warning(error.message);
}
const conclusion = verdict.conclusion === 'success'
? 'success'
: 'failure';
const summary = JSON.stringify(verdict);
await github.rest.checks.create({
owner: context.repo.owner,
repo: context.repo.repo,
name: 'Agent completion enforcement',
head_sha: pull.data.head.sha,
status: 'completed',
conclusion,
output: {
title: conclusion === 'success'
? 'Trusted evidence verified'
: 'Trusted evidence blocked',
summary: summary.slice(0, 60000)
}
});
if (conclusion !== 'success') {
core.setFailed(verdict.reason || 'trusted evidence blocked');
}
github-***REDACTED_SECRET_ASSIGNMENT***
debug: false
user-agent: actions/github-script
result-encoding: json
retries: 0
retry-exempt-status-codes: 400,401,403,404,422
env:
PR: 1205
##[endgroup]
##[error]missing_trusted_publication
GitHub Actions: 🔍 Dependency Review / dependency-review: perf(cloud-ai): run AWS Rekognition boto3 calls off the event loop
Conclusion: failure
##[group]Run actions/dependency-review-action@v5
with:
fail-on-severity: moderate
allow-ghsas: GHSA-w5hq-g745-h8pq
allow-licenses: MIT, Apache-2.0, BSD-2-Clause, BSD-3-Clause, ISC, Python-2.0, BlueOak-1.0.0, MPL-2.0, CC0-1.0, 0BSD
allow-dependencies-licenses: pkg:npm/caniuse-lite, pkg:npm/@sentry/nextjs, pkg:npm/@sentry/cli, pkg:npm/@sentry/cli-darwin, pkg:npm/@sentry/cli-linux-arm, pkg:npm/@sentry/cli-linux-arm64, pkg:npm/@sentry/cli-linux-i686, pkg:npm/@sentry/cli-linux-x64, pkg:npm/@sentry/cli-win32-arm64, pkg:npm/@sentry/cli-win32-i686, pkg:npm/@sentry/cli-win32-x64, pkg:npm/@sentry/bundler-plugin-core, pkg:npm/@sentry/babel-plugin-component-annotate
comment-summary-in-pr: always
repo-***REDACTED_SECRET_ASSIGNMENT***
##[endgroup]
##[error]Dependency review is not supported on this repository. Please ensure that Dependency graph is enabled, see https://github.com/groupthinking/EventRelay/settings/security_analysis
GitHub Actions: 🔍 Dependency Review / 0_dependency-review.txt: perf(cloud-ai): run AWS Rekognition boto3 calls off the event loop
Conclusion: failure
##[group]Run actions/dependency-review-action@v5
with:
fail-on-severity: moderate
allow-ghsas: GHSA-w5hq-g745-h8pq
allow-licenses: MIT, Apache-2.0, BSD-2-Clause, BSD-3-Clause, ISC, Python-2.0, BlueOak-1.0.0, MPL-2.0, CC0-1.0, 0BSD
allow-dependencies-licenses: pkg:npm/caniuse-lite, pkg:npm/@sentry/nextjs, pkg:npm/@sentry/cli, pkg:npm/@sentry/cli-darwin, pkg:npm/@sentry/cli-linux-arm, pkg:npm/@sentry/cli-linux-arm64, pkg:npm/@sentry/cli-linux-i686, pkg:npm/@sentry/cli-linux-x64, pkg:npm/@sentry/cli-win32-arm64, pkg:npm/@sentry/cli-win32-i686, pkg:npm/@sentry/cli-win32-x64, pkg:npm/@sentry/bundler-plugin-core, pkg:npm/@sentry/babel-plugin-component-annotate
comment-summary-in-pr: always
repo-***REDACTED_SECRET_ASSIGNMENT***
##[endgroup]
##[error]Dependency review is not supported on this repository. Please ensure that Dependency graph is enabled, see https://github.com/groupthinking/EventRelay/settings/security_analysis
GitHub Actions: Secret Scan / 0_gitleaks (working tree).txt: perf(cloud-ai): run AWS Rekognition boto3 calls off the event loop
Conclusion: failure
##[group]Run gitleaks detect --no-git --config .gitleaks.toml --redact --verbose --exit-code 1
�[36;1mgitleaks detect --no-git --config .gitleaks.toml --redact --verbose --exit-code 1�[0m
shell: /usr/bin/bash -e {0}
##[endgroup]
○
│╲
│ ○
○ ░
░ gitleaks
Finding: ...gz", hash = "sha256:�[1;3;mREDACTED�[0m, size = 401824, upl...
***REDACTED_SECRET_ASSIGNMENT***
RuleID: square-access-token
Entropy: 3.884400
File: uv.lock
Line: 5129
Fingerprint: uv.lock:square-access-***REDACTED_SECRET_ASSIGNMENT***
�[90m11:06PM�[0m �[32mINF�[0m scan completed in 5.87s
�[90m11:06PM�[0m �[31mWRN�[0m leaks found: 1
##[error]Process completed with exit code 1.
GitHub Actions: Secret Scan / gitleaks (working tree): perf(cloud-ai): run AWS Rekognition boto3 calls off the event loop
Conclusion: failure
##[group]Run gitleaks detect --no-git --config .gitleaks.toml --redact --verbose --exit-code 1
�[36;1mgitleaks detect --no-git --config .gitleaks.toml --redact --verbose --exit-code 1�[0m
shell: /usr/bin/bash -e {0}
##[endgroup]
○
│╲
│ ○
○ ░
░ gitleaks
Finding: ...gz", hash = "sha256:�[1;3;mREDACTED�[0m, size = 401824, upl...
***REDACTED_SECRET_ASSIGNMENT***
RuleID: square-access-token
Entropy: 3.884400
File: uv.lock
Line: 5129
Fingerprint: uv.lock:square-access-***REDACTED_SECRET_ASSIGNMENT***
�[90m11:06PM�[0m �[32mINF�[0m scan completed in 5.87s
�[90m11:06PM�[0m �[31mWRN�[0m leaks found: 1
##[error]Process completed with exit code 1.
GitHub Actions: Coverage / Generate and Upload Coverage: perf(cloud-ai): run AWS Rekognition boto3 calls off the event loop
Conclusion: failure
ion PASSED [ 96%]
tests/unit/test_video_utils.py::TestParseDurationToSeconds::test_minutes_and_seconds PASSED [ 96%]
tests/unit/test_video_utils.py::TestParseDurationToSeconds::test_seconds_only PASSED [ 96%]
tests/unit/test_video_utils.py::TestParseDurationToSeconds::test_hours_and_minutes PASSED [ 96%]
tests/unit/test_video_utils.py::TestParseDurationToSeconds::test_hours_only PASSED [ 96%]
tests/unit/test_video_utils.py::TestParseDurationToSeconds::test_minutes_only PASSED [ 96%]
tests/unit/test_video_utils.py::TestParseDurationToSeconds::test_invalid_returns_zero PASSED [ 96%]
tests/unit/test_video_utils.py::TestParseDurationToSeconds::test_zero_duration PASSED [ 96%]
tests/unit/test_video_utils.py::TestParseDurationToSeconds::test_large_hours PASSED [ 96%]
tests/unit/test_video_utils.py::TestFormatDuration::test_full_duration PASSED [ 96%]
tests/unit/test_video_utils.py::TestFormatDuration::test_minutes_and_seconds PASSED [ 96%]
tests/unit/test_video_utils.py::TestFormatDuration::test_seconds_only PASSED [ 96%]
tests/unit/test_video_utils.py::TestFormatDuration::test_hours_and_minutes PASSED [ 96%]
tests/unit/test_video_utils.py::TestFormatDuration::test_hours_only PASSED [ 96%]
tests/unit/test_video_utils.py::TestFormatDuration::test_minutes_only PASSED [ 96%]
tests/unit/test_video_utils.py::TestFormatDuration::test_zero_seconds PASSED [ 96%]
tests/unit/test_video_utils.py::TestFormatDuration::test_invalid_returns_original PASSED [ 96%]
tests/unit/test_videopack.py::TestVPVersion::test_v0_value PASSED [ 96%]
tests/unit/test_videopack.py::TestTranscriptSegment::test_basic_creation PASSED [ 96%]
tests/unit/test_videopack.py::TestTranscriptSegment::test_idx_stored PASSED [ 96%]
tests/unit/test_videopack.py::TestTranscript::test_full_text_stored PASSED [ 97%]
tests/unit/test_videopack.py::TestTranscript::test_segments_default_empty PASSED [ 97%]
tests/unit/test_videopack.py::TestTranscript::test_language_optional PASSED [ 97%]
test...
GitHub Actions: Coverage / 0_Generate and Upload Coverage.txt: perf(cloud-ai): run AWS Rekognition boto3 calls off the event loop
Conclusion: failure
ion PASSED [ 96%]
tests/unit/test_video_utils.py::TestParseDurationToSeconds::test_minutes_and_seconds PASSED [ 96%]
tests/unit/test_video_utils.py::TestParseDurationToSeconds::test_seconds_only PASSED [ 96%]
tests/unit/test_video_utils.py::TestParseDurationToSeconds::test_hours_and_minutes PASSED [ 96%]
tests/unit/test_video_utils.py::TestParseDurationToSeconds::test_hours_only PASSED [ 96%]
tests/unit/test_video_utils.py::TestParseDurationToSeconds::test_minutes_only PASSED [ 96%]
tests/unit/test_video_utils.py::TestParseDurationToSeconds::test_invalid_returns_zero PASSED [ 96%]
tests/unit/test_video_utils.py::TestParseDurationToSeconds::test_zero_duration PASSED [ 96%]
tests/unit/test_video_utils.py::TestParseDurationToSeconds::test_large_hours PASSED [ 96%]
tests/unit/test_video_utils.py::TestFormatDuration::test_full_duration PASSED [ 96%]
tests/unit/test_video_utils.py::TestFormatDuration::test_minutes_and_seconds PASSED [ 96%]
tests/unit/test_video_utils.py::TestFormatDuration::test_seconds_only PASSED [ 96%]
tests/unit/test_video_utils.py::TestFormatDuration::test_hours_and_minutes PASSED [ 96%]
tests/unit/test_video_utils.py::TestFormatDuration::test_hours_only PASSED [ 96%]
tests/unit/test_video_utils.py::TestFormatDuration::test_minutes_only PASSED [ 96%]
tests/unit/test_video_utils.py::TestFormatDuration::test_zero_seconds PASSED [ 96%]
tests/unit/test_video_utils.py::TestFormatDuration::test_invalid_returns_original PASSED [ 96%]
tests/unit/test_videopack.py::TestVPVersion::test_v0_value PASSED [ 96%]
tests/unit/test_videopack.py::TestTranscriptSegment::test_basic_creation PASSED [ 96%]
tests/unit/test_videopack.py::TestTranscriptSegment::test_idx_stored PASSED [ 96%]
tests/unit/test_videopack.py::TestTranscript::test_full_text_stored PASSED [ 97%]
tests/unit/test_videopack.py::TestTranscript::test_segments_default_empty PASSED [ 97%]
tests/unit/test_videopack.py::TestTranscript::test_language_optional PASSED [ 97%]
test...
GitHub Actions: CI / guards: perf(cloud-ai): run AWS Rekognition boto3 calls off the event loop
Conclusion: failure
##[group]Run # Opening/closing conflict sentinels always carry a label after the
�[36;1m# Opening/closing conflict sentinels always carry a label after the�[0m
�[36;1m# space, so this never matches decorative "=======" underlines.�[0m
�[36;1mif git grep -nE '^(<<<<<<<|>>>>>>>) ' -- . ':(exclude).github/workflows/ci.yml'; then�[0m
�[36;1m echo "::error::Committed merge-conflict markers found (see matches above)."�[0m
GitHub Actions: CI / guards: perf(cloud-ai): run AWS Rekognition boto3 calls off the event loop
Conclusion: failure
##[group]Run # VS Code forks (Antigravity, Cursor, Windsurf) write their own
�[36;1m# VS Code forks (Antigravity, Cursor, Windsurf) write their own�[0m
�[36;1m# extension IDs into workspace settings; those IDs resolve to�[0m
�[36;1m# nothing in stock VS Code and fail silently. Mirrors the�[0m
�[36;1m# vscode-ide-self-reference pre-commit hook, which not every�[0m
�[36;1m# committer has installed.�[0m
�[36;1mif git grep -nE 'google\.antigravity|anysphere\.|codeium\.windsurf' -- .vscode/; then�[0m
�[36;1m echo "::error::IDE self-identifier found in shared .vscode/ config (see matches above)."�[0m
GitHub Actions: CI / 3_lint-python.txt: perf(cloud-ai): run AWS Rekognition boto3 calls off the event loop
Conclusion: failure
##[group]Run ruff check src/youtube_extension/backend/ src/youtube_extension/main.py --ignore E402,F811,F401,F821,B904,B020,E701,E722
�[36;1mruff check src/youtube_extension/backend/ src/youtube_extension/main.py --ignore E402,F811,F401,F821,B904,B020,E701,E722�[0m
shell: /usr/bin/bash -e {0}
env:
pythonLocation: /opt/hostedtoolcache/Python/3.12.13/x64
PKG_CONFIG_PATH: /opt/hostedtoolcache/Python/3.12.13/x64/lib/pkgconfig
Python_ROOT_DIR: /opt/hostedtoolcache/Python/3.12.13/x64
Python2_ROOT_DIR: /opt/hostedtoolcache/Python/3.12.13/x64
Python3_ROOT_DIR: /opt/hostedtoolcache/Python/3.12.13/x64
LD_LIBRARY_PATH: /opt/hostedtoolcache/Python/3.12.13/x64/lib
##[endgroup]
UP035 `typing.Dict` is deprecated, use `dict` instead
--> src/youtube_extension/backend/deploy/__init__.py:3:1
|
1 | import importlib
2 | from collections.abc import Awaitable
3 | from typing import Any, Callable, Dict
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
4 |
5 | AdapterFunc = Callable[[str, dict[str, Any], dict[str, Any]], Awaitable[dict[str, Any]]]
|
B025 try-except block with duplicate exception `Exception`
--> src/youtube_extension/backend/services/data_service.py:340:16
|
338 | return None
339 |
340 | except Exception as e:
| ^^^^^^^^^
341 | logger.error(f"Error reading video detail: {e}")
342 | return None
|
Found 2 errors.
##[error]Process completed with exit code 1.
GitHub Actions: CI / 2_guards.txt: perf(cloud-ai): run AWS Rekognition boto3 calls off the event loop
Conclusion: failure
##[group]Run # Opening/closing conflict sentinels always carry a label after the
�[36;1m# Opening/closing conflict sentinels always carry a label after the�[0m
�[36;1m# space, so this never matches decorative "=======" underlines.�[0m
�[36;1mif git grep -nE '^(<<<<<<<|>>>>>>>) ' -- . ':(exclude).github/workflows/ci.yml'; then�[0m
�[36;1m echo "::error::Committed merge-conflict markers found (see matches above)."�[0m
GitHub Actions: CI / lint-python: perf(cloud-ai): run AWS Rekognition boto3 calls off the event loop
Conclusion: failure
##[group]Run ruff check src/youtube_extension/backend/ src/youtube_extension/main.py --ignore E402,F811,F401,F821,B904,B020,E701,E722
�[36;1mruff check src/youtube_extension/backend/ src/youtube_extension/main.py --ignore E402,F811,F401,F821,B904,B020,E701,E722�[0m
shell: /usr/bin/bash -e {0}
env:
pythonLocation: /opt/hostedtoolcache/Python/3.12.13/x64
PKG_CONFIG_PATH: /opt/hostedtoolcache/Python/3.12.13/x64/lib/pkgconfig
Python_ROOT_DIR: /opt/hostedtoolcache/Python/3.12.13/x64
Python2_ROOT_DIR: /opt/hostedtoolcache/Python/3.12.13/x64
Python3_ROOT_DIR: /opt/hostedtoolcache/Python/3.12.13/x64
LD_LIBRARY_PATH: /opt/hostedtoolcache/Python/3.12.13/x64/lib
##[endgroup]
UP035 `typing.Dict` is deprecated, use `dict` instead
--> src/youtube_extension/backend/deploy/__init__.py:3:1
|
1 | import importlib
2 | from collections.abc import Awaitable
3 | from typing import Any, Callable, Dict
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
4 |
5 | AdapterFunc = Callable[[str, dict[str, Any], dict[str, Any]], Awaitable[dict[str, Any]]]
|
B025 try-except block with duplicate exception `Exception`
--> src/youtube_extension/backend/services/data_service.py:340:16
|
338 | return None
339 |
340 | except Exception as e:
| ^^^^^^^^^
341 | logger.error(f"Error reading video detail: {e}")
342 | return None
|
Found 2 errors.
##[error]Process completed with exit code 1.
GitHub Actions: CI / test: perf(cloud-ai): run AWS Rekognition boto3 calls off the event loop
Conclusion: failure
s_not_propagate
-------------------------------- live log call ---------------------------------
[ WARNING] youtube_extension.backend.services.video_processing_service: Processor cleanup warning: close error
PASSED [ 95%]
tests/unit/test_video_processor_factory.py::TestAutoMode::test_auto_defaults_to_enhanced_when_no_env
-------------------------------- live log call ---------------------------------
[ INFO] youtube_extension.backend.video_processor_factory: Creating video processor: enhanced
[ INFO] youtube_extension.backend.video_processor_factory: ✅ Using EnhancedVideoProcessor (Gemini + YouTube API)
PASSED [ 95%]
tests/unit/test_video_processor_factory.py::TestAutoMode::test_auto_env_var_enhanced
-------------------------------- live log call ---------------------------------
[ INFO] youtube_extension.backend.video_processor_factory: Creating video processor: enhanced
[ INFO] youtube_extension.backend.video_processor_factory: ✅ Using EnhancedVideoProcessor (Gemini + YouTube API)
PASSED [ 95%]
tests/unit/test_video_processor_factory.py::TestAutoMode::test_auto_env_var_real
-------------------------------- live log call ---------------------------------
[ INFO] youtube_extension.backend.video_processor_factory: Creating video processor: real
[ INFO] youtube_extension.backend.video_processor_factory: ✅ Using RealVideoProcessor (MCP ecosystem)
PASSED [ 95%]
tests/unit/test_video_processor_factory.py::TestEnhancedProcessorType::test_returns_enhanced_processor
-------------------------------- live log call ---------------------------------
[ INFO] youtube_extension.backend.video_processor_factory: Creating video processor: enhanced
[ INFO] youtube_exte...
GitHub Actions: CI / 1_test.txt: perf(cloud-ai): run AWS Rekognition boto3 calls off the event loop
Conclusion: failure
s_not_propagate
-------------------------------- live log call ---------------------------------
[ WARNING] youtube_extension.backend.services.video_processing_service: Processor cleanup warning: close error
PASSED [ 95%]
tests/unit/test_video_processor_factory.py::TestAutoMode::test_auto_defaults_to_enhanced_when_no_env
-------------------------------- live log call ---------------------------------
[ INFO] youtube_extension.backend.video_processor_factory: Creating video processor: enhanced
[ INFO] youtube_extension.backend.video_processor_factory: ✅ Using EnhancedVideoProcessor (Gemini + YouTube API)
PASSED [ 95%]
tests/unit/test_video_processor_factory.py::TestAutoMode::test_auto_env_var_enhanced
-------------------------------- live log call ---------------------------------
[ INFO] youtube_extension.backend.video_processor_factory: Creating video processor: enhanced
[ INFO] youtube_extension.backend.video_processor_factory: ✅ Using EnhancedVideoProcessor (Gemini + YouTube API)
PASSED [ 95%]
tests/unit/test_video_processor_factory.py::TestAutoMode::test_auto_env_var_real
-------------------------------- live log call ---------------------------------
[ INFO] youtube_extension.backend.video_processor_factory: Creating video processor: real
[ INFO] youtube_extension.backend.video_processor_factory: ✅ Using RealVideoProcessor (MCP ecosystem)
PASSED [ 95%]
tests/unit/test_video_processor_factory.py::TestEnhancedProcessorType::test_returns_enhanced_processor
-------------------------------- live log call ---------------------------------
[ INFO] youtube_extension.backend.video_processor_factory: Creating video processor: enhanced
[ INFO] youtube_exte...
Commit Status: Vercel: Vercel
Conclusion: failure
Canceled from the Vercel Dashboard
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{py,js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{py,js,jsx,ts,tsx}: Use Python 3.9+ and Node 18+ for development
Never hardcode API keys, database URLs, or secrets in code
Make minimal, surgical changes and avoid deleting working code unless fixing security issues
Files:
src/youtube_extension/integrations/cloud_ai/providers/aws_rekognition.py
**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.py: Always use type hints for Python functions
Use Black formatter with 88 character line length for Python code
Follow PEP 8 conventions for Python code
Use descriptive variable names and add docstrings to all public functions in Python
Group imports in Python: standard library, third-party, local
Use SQLAlchemy ORM and never write raw SQL queries
Use environment variables via os.getenv() or pydantic-settings to access configuration
Wrap database operations in try-except blocks and use context managers or FastAPI dependencies for connection cleanup
Implement comprehensive error handling with proper logging in all functions
Version APIs using /api/v1/ prefix for stability
Use JSON-RPC 2.0 protocol for all MCP communication
Follow the single-flow workflow: YouTube link → context extraction → agent dispatch → outputs
Use context managers for resource management in Python code
Implement comprehensive input validation and sanitize outputs for security
Use parameterized queries and SQLAlchemy ORM to prevent SQL injection
Define API request/response models using Pydantic for FastAPI endpoints
Store the single unified workflow as the only workflow; never introduce alternate flows or manual triggers
Use SQLAlchemy with connection pooling for database connections and manage sessions with context managers
Provide sensible defaults for non-sensitive configuration in Python settings
Maintain backward compatibility and do not break existing API endpoints
Include comprehensive logging for debugging in MCP implementations
Files:
src/youtube_extension/integrations/cloud_ai/providers/aws_rekognition.py
⚙️ CodeRabbit configuration file
Python backend code. Check for type hints, proper exception handling, async context manager usage, and potential blocking calls in async functions. Flag any bare except clauses or missing timeout parameters on network calls. CRITICAL: Flag any file that contains placeholder/stub implementations — especially in unified_ai_sdk. Any class or function that says "TODO: Replace with production implementation" or returns mock/fake data must be flagged as a blocking issue. Flag any code generation output that reaches users without AST validation or syntax checking.
Files:
src/youtube_extension/integrations/cloud_ai/providers/aws_rekognition.py
**/*.{py,js,ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Maintain >80% code coverage for new features
Files:
src/youtube_extension/integrations/cloud_ai/providers/aws_rekognition.py
**/*.{py,ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{py,ts,tsx}: Keep frontend and backend data models synchronized using matching Pydantic (backend) and TypeScript (frontend) interfaces
Use type-safe interfaces for backend-frontend data exchange
**/*.{py,ts,tsx}: Production code must use real behavior only: no mock delays, fake data, or simulated responses.
Maintain strict type safety: mypy strict mode for Python and TypeScript strict mode for the frontend.
Name events using the<domain>.<entity>.<action>format.
Files:
src/youtube_extension/integrations/cloud_ai/providers/aws_rekognition.py
**/*
📄 CodeRabbit inference engine (Custom checks)
**/*: Strictly verify that GitHub Copilot has explicitly reviewed and approved the pull request; human approvals alone must not satisfy this check.
Before allowing a merge, require thecopilot-rabbitlabel and AI-generated unit tests committed alongside the code changes; fail the check if either is missing.For Vercel-specific work, include
https://vercel.com/docs/llms-full.txtin the AI assistant context set.
Files:
src/youtube_extension/integrations/cloud_ai/providers/aws_rekognition.py
**/*.{py,pyw}
📄 CodeRabbit inference engine (AGENTS.md)
Write Python code to remain compatible with Linux and Windows where possible, including correct handling of
asyncioevent loops.
Files:
src/youtube_extension/integrations/cloud_ai/providers/aws_rekognition.py
src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.py: Format Python code with Black using an 88-character line length.
Sort Python imports with isort using the Black profile.
Run Ruff with E, W, F, I, B, C4, and UP rules; E501 is ignored.
Use mypy strict mode; untyped function definitions are disallowed.
Target Python 3.9 or newer.
Validate backend inputs with Pydantic and sanitize subprocess arguments.
Use Anthropic SDK featuresthinking={"type": "adaptive"}andoutput_config={"effort": "..."}withanthropic>=0.105.0; do not addTypeErrorfallbacks for these parameters.Use the service container dependency-injection pattern in
backend/containers/.
Files:
src/youtube_extension/integrations/cloud_ai/providers/aws_rekognition.py
**/*.{py,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not commit secrets; store keys and credentials in gitignored
.envfiles.
Files:
src/youtube_extension/integrations/cloud_ai/providers/aws_rekognition.py
**/*.{py,pyi}
📄 CodeRabbit inference engine (GEMINI.md)
**/*.{py,pyi}: Use Black formatting with an 88-character line length for Python code.
Use Ruff with rules E, W, F, I, B, C4, and UP for Python linting.
Use strict mypy type checking; do not define untyped functions.
Target Python 3.9 or newer.
Validate Python inputs with Pydantic.
Sanitize subprocess arguments before execution.
Do not add mock delays or fake data to production Python code; production runs in REAL_MODE_ONLY.
Keep secrets out of Python source code; load keys and credentials from environment variables instead.
Use absolute imports that resolve withPYTHONPATH=srcin the Python backend.
Files:
src/youtube_extension/integrations/cloud_ai/providers/aws_rekognition.py
**/*.{py,pyi,ts,tsx}
📄 CodeRabbit inference engine (GEMINI.md)
**/*.{py,pyi,ts,tsx}: Preserve the single workflow: YouTube link → transcript → events → agents → outputs; do not introduce alternative flows or manual triggers that bypass it.
Use event names following<domain>.<entity>.<action>, such asyoutube.video.captured.
Make surgical, precise changes and do not delete working code without justification.
Files:
src/youtube_extension/integrations/cloud_ai/providers/aws_rekognition.py
🪛 ast-grep (0.45.0)
src/youtube_extension/integrations/cloud_ai/providers/aws_rekognition.py
[warning] 35-35: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(path, 'rb')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
🔍 Remote MCP GitHub Copilot
Relevant review context
- Issue
#1204enumerates 14 blocking Rekognition calls plus a synchronous local-file read. The PR wraps all listed calls withasyncio.to_threadand changes only the provider and its unit test. AWSRekognition.cleanup()only sets client references toNone; the integrator awaits provider cleanup. This is relevant when assessing cancellation of in-flight worker calls.- The new responsiveness tests cover label detection, one video-start path, one polling path, and local reads. The aggregate image test does not independently prove that the other three detection calls are off-loop; an unresolved review comment requests isolated coverage for all 14 call sites.
- The PR’s current checks include failures for
test, coverage generation, gitleaks, dependency review, and agent-completion enforcement; Python lint, CodeQL, Python security scanning, build, and guards are reported successful. The available check metadata does not identify failure causes. - Merged PR
#1190used a cancellation-aware wrapper because its shared client could be closed during cancellation. That differs from this provider’s cleanup implementation, which does not close a transport.
🔇 Additional comments (1)
src/youtube_extension/integrations/cloud_ai/providers/aws_rekognition.py (1)
185-210: 🎯 Functional CorrectnessVerify isolated responsiveness coverage for every offloaded call branch.
The implementation has 14 independent boto3 call sites. The supplied test evidence covers an aggregate image path, one video-submission path, and one polling path. A different branch could regress to direct boto3 execution while those tests still pass. Add parameterized tests for each client method, or assert the worker thread identity for every stubbed branch.
As per coding guidelines, maintain more than 80% code coverage for new features. As per PR objectives, Issue
#1204requires responsiveness tests that fail against the original implementation.Also applies to: 294-320, 346-364
Source: Coding guidelines
|
@coderabbitai review Please re-review exact head |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
target='af04aedb59d5b46871c46ab945536faa58c97d91'
provider='src/youtube_extension/integrations/cloud_ai/providers/aws_rekognition.py'
test_file='tests/unit/test_aws_rekognition_provider.py'
echo '=== Exact commit verification ==='
git rev-parse HEAD
git cat-file -t "$target"
git show -s --format='commit=%H%nparents=%P%nsubject=%s' "$target"
git merge-base --is-ancestor "$target" HEAD && echo 'target_is_ancestor_of_HEAD=true' || echo 'target_is_ancestor_of_HEAD=false'
parent="$(git show -s --format=%P "$target" | awk '{print $1}')"
echo "=== Diff: ${parent}..${target} ==="
git diff --stat "$parent" "$target"
git diff --unified=45 "$parent" "$target" -- "$provider" "$test_file"
echo '=== Provider structure ==='
ast-grep outline "$provider" --items all --type function,class --view expanded
echo '=== All async thread dispatches and Rekognition call contexts ==='
rg -n -C 6 --type py \
'await\s+asyncio\.to_thread|_rekognition_client\.(describe_collection|detect_labels|detect_faces|detect_text|detect_moderation_labels|start_[a-z_]+_detection|get_[a-z_]+_detection)|def\s+_read_file_bytes|async\s+def\s+_prepare_image_input' \
"$provider"
echo '=== Test structure and all off-loop/local-read coverage contexts ==='
ast-grep outline "$test_file" --items all --type function,class --view expanded
rg -n -C 12 --type py \
'to_thread|threading\.get_ident|_read_file_bytes|sys\.modules|importlib|monkeypatch|patch\.object|heartbeat|ticks|describe_collection|detect_labels|detect_faces|detect_text|detect_moderation_labels|start_[a-z_]+_detection|get_[a-z_]+_detection' \
"$test_file"
echo '=== Count actual await asyncio.to_thread call sites ==='
ast-grep run --pattern 'await asyncio.to_thread($$$ARGS)' --lang python "$provider"
echo '=== Test file path discovery if expected path changed ==='
fd -HI -t f '.*aws.*rekognition.*\.py$' . | sortLength of output: 15553 🏁 Script executed: #!/bin/bash
set -euo pipefail
target='af04aedb59d5b46871c46ab945536faa58c97d91'
base='origin/main'
test_file='tests/unit/test_aws_rekognition_provider.py'
provider='src/youtube_extension/integrations/cloud_ai/providers/aws_rekognition.py'
echo '=== Target tree entries for Rekognition tests ==='
git ls-tree -r --name-only "$target" | rg -i '(^|/).*aws.*rekognition.*\.py$|rekognition.*test.*\.py$' || true
echo '=== Confirm target objects ==='
git cat-file -e "${target}:${test_file}"
git cat-file -e "${target}:${provider}"
echo 'target_contains_expected_test_and_provider=true'
echo '=== PR-range changed test methods and helpers ==='
git diff --unified=30 "${base}...${target}" -- "$test_file"
echo '=== Target test method names ==='
git show "${target}:${test_file}" | rg -n \
'^\s*(async\s+def|def)\s+test_|^\s*(async\s+def|def)\s+_count_heartbeats|^\s*(async\s+def|def)\s+_slow|parametrize'
echo '=== Target test contexts: coverage helpers and isolated SDK operations ==='
git show "${target}:${test_file}" | rg -n -C 14 \
'_count_heartbeats|def _slow|test_each_|test_every_|test_local_image|describe_collection|detect_labels|detect_faces|detect_text|detect_moderation_labels|start_label_detection|start_face_detection|start_text_detection|start_content_moderation|get_label_detection|get_face_detection|get_text_detection|get_content_moderation|method_globals|patch\.dict|__globals__'
echo '=== Exact local-read test from target ==='
git show "${target}:${test_file}" | sed -n '/async def test_local_image_read_runs_off_the_event_loop_thread/,/async def test_local_image_bytes_are_read_correctly/p'
echo '=== Exact provider dispatches from target ==='
git show "${target}:${provider}" | rg -n -C 3 \
'await asyncio\.to_thread|_read_file_bytes|describe_collection|detect_(labels|faces|text|moderation_labels)|start_(label|face|text|content_moderation)_detection|get_(label|face|text|content_moderation)(_detection)?'Length of output: 54701
I found no new blocking defect in this test repair. The off-loop coverage now isolates all 14 Rekognition SDK paths:
Each case uses one slow mocked operation. A direct synchronous regression in any one path makes that case report zero heartbeats. The local-read test now patches The three unresolved provider findings remain unresolved. This commit does not change those production paths. No additional finding applies to this focused test repair. ✅ Action performedReview finished.
|
|
@coderabbitai review Head is now Please challenge specifically:
|
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
target='af04aedb5'
provider='src/youtube_extension/integrations/cloud_ai/providers/aws_rekognition.py'
test_file='tests/unit/test_aws_rekognition_provider.py'
integrator='src/youtube_extension/integrations/cloud_ai/integrator.py'
echo '=== Exact commit ==='
git rev-parse "$target"
git show -s --format='commit=%H%nparents=%P%nsubject=%s' "$target"
git merge-base --is-ancestor "$target" HEAD && echo 'target_is_ancestor_of_HEAD=true' || echo 'target_is_ancestor_of_HEAD=false'
echo '=== Changed paths at target parent ==='
parent="$(git show -s --format=%P "$target" | awk '{print $1}')"
git diff --name-status "$parent" "$target"
echo '=== Provider structure ==='
ast-grep outline "$provider" --items all --type class,function --view expanded
echo '=== Provider: class, cleanup, file-read, and polling paths ==='
rg -n -C 14 --type py \
'class\s+AWSRekognition|def\s+_read_file_bytes|async\s+def\s+_prepare_image_input|async\s+def\s+_wait_for_job_completion|async\s+def\s+cleanup|await\s+asyncio\.to_thread|await\s+asyncio\.sleep|_rekognition_client\.' \
"$provider"
echo '=== Focused test methods and namespace patch ==='
ast-grep outline "$test_file" --items all --type function,class --view expanded
rg -n -C 16 --type py \
'method_globals|__globals__|patch\.dict|_read_file_bytes|type\(provider\)\._prepare_image_input|test_.*off.*loop|heartbeat|ticks|revert|parametrize' \
"$test_file"
echo '=== Current exact target versions of critical regions ==='
git show "${target}:${provider}" | sed -n '1,430p'
git show "${target}:${test_file}" | sed -n '340,520p'
echo '=== Integrator cleanup lifecycle ==='
sed -n '35,125p' "$integrator"Length of output: 15919 🏁 Script executed: #!/bin/bash
set -euo pipefail
target='af04aedb59d5b46871c46ab945536faa58c97d91'
provider='src/youtube_extension/integrations/cloud_ai/providers/aws_rekognition.py'
echo '=== Rekognition-related Python files in exact target tree ==='
git ls-tree -r --name-only "$target" | rg -i '(^|/).*rekognition.*\.py$|(^|/).*aws.*\.py$' || true
test_file="$(
git ls-tree -r --name-only "$target" |
rg -i '(^|/).*rekognition.*test.*\.py$|(^|/).*test.*rekognition.*\.py$' |
head -n 1
)"
if [ -z "$test_file" ]; then
echo 'ERROR: no Rekognition test file found in target tree' >&2
exit 1
fi
echo "test_file=$test_file"
echo '=== Test structure in exact target ==='
git show "${target}:${test_file}" > /tmp/rekognition-test.py
ast-grep outline /tmp/rekognition-test.py --items all --type function,class --view expanded
rg -n -C 16 --type py \
'method_globals|__globals__|patch\.dict|_read_file_bytes|type\(provider\)\._prepare_image_input|heartbeat|ticks|parametrize|slow|describe_collection|detect_|start_|get_' \
/tmp/rekognition-test.py
echo '=== Exact local-read and per-operation tests ==='
sed -n '330,560p' /tmp/rekognition-test.py
echo '=== Full polling method in exact target ==='
git show "${target}:${provider}" | sed -n '325,385p'
echo '=== Integrator cleanup context ==='
integrator_path="$(
git ls-tree -r --name-only "$target" |
rg '(^|/)integrator\.py$' |
rg 'cloud_ai' |
head -n 1
)"
echo "integrator_path=$integrator_path"
git show "${target}:${integrator_path}" | sed -n '35,130p'Length of output: 61990
No changes requested. ✅ Action performedReview finished.
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
target='b5721eb4f'
provider='src/youtube_extension/integrations/cloud_ai/providers/aws_rekognition.py'
test_file='tests/unit/test_aws_rekognition_provider.py'
echo '=== Exact target ==='
git rev-parse "$target"
git show -s --format='commit=%H%nparents=%P%nsubject=%s' "$target"
git merge-base --is-ancestor "$target" HEAD && echo 'target_is_ancestor_of_HEAD=true' || echo 'target_is_ancestor_of_HEAD=false'
parent="$(git show -s --format=%P "$target" | awk '{print $1}')"
echo '=== Commit diff ==='
git diff --stat "$parent" "$target"
git diff --unified=50 "$parent" "$target" -- "$provider" "$test_file"
echo '=== Provider outline ==='
ast-grep outline "$provider" --items all --type class,function --view expanded
echo '=== Timeout, retry, initialization, polling, and cleanup contexts ==='
rg -n -C 14 --type py \
'Config\(|connect_timeout|read_timeout|max_attempts|retry|os\.getenv|math\.isfinite|ConfigurationError|async\s+def\s+initialize|except\s+Exception|async\s+def\s+_wait_for_job_completion|asyncio\.sleep|asyncio\.to_thread|async\s+def\s+cleanup' \
"$provider"
echo '=== Related tests and mutation coverage ==='
rg -n -C 12 --type py \
'timeout|retry|ConfigurationError|connect_timeout|read_timeout|max_attempts|finite|nan|inf|mutation|off_the_loop|heartbeat' \
"$test_file"
echo '=== Configuration error definitions and provider initialization callers ==='
rg -n -C 6 --type py \
'class\s+ConfigurationError|ConfigurationError|await\s+.*\.initialize\s*\(|\.initialize\s*\(' \
src
echo '=== GitHub issue `#1209` ==='
gh issue view 1209 --repo groupthinking/EventRelay --json number,title,state,body,urlLength of output: 45065 🏁 Script executed: #!/bin/bash
set -euo pipefail
target='b5721eb4fc1327611a17664659051cab1fb7dd09'
provider='src/youtube_extension/integrations/cloud_ai/providers/aws_rekognition.py'
echo '=== Locate Rekognition tests in target tree ==='
git ls-tree -r --name-only "$target" | rg -i '(^|/).*rekognition.*\.py$|(^|/).*aws.*rekognition.*\.py$' || true
test_file="$(
git ls-tree -r --name-only "$target" |
rg -i '(^|/)(test_)?aws_rekognition(_provider)?\.py$|(^|/).*test.*rekognition.*\.py$' |
head -n 1
)"
test -n "$test_file"
echo "test_file=$test_file"
echo '=== Timeout tests in exact target ==='
git show "${target}:${test_file}" | rg -n -C 14 \
'TestClientTimeoutConfiguration|_initialize|connect_timeout|read_timeout|max_attempts|mode|ConfigurationError|math\.inf|finite|blank|environment|mutation'
echo '=== Exception definitions and initialization call contexts ==='
rg -n -C 8 --type py \
'class\s+ConfigurationError|class\s+CloudAIError|await\s+.*\.initialize\s*\(|\.initialize\s*\(' \
src
echo '=== Relevant exact target provider lines ==='
git show "${target}:${provider}" | sed -n '36,205p'
echo '=== GitHub issue `#1209` ==='
gh issue view 1209 --repo groupthinking/EventRelay --json number,title,state,body,urlLength of output: 69728 🤖 web_search executed:
Length of output: 16925 🏁 Script executed: #!/bin/bash
set -euo pipefail
target='b5721eb4fc1327611a17664659051cab1fb7dd09'
test_file='tests/unit/test_aws_rekognition_provider.py'
echo '=== Exact timeout tests ==='
git show "${target}:${test_file}" | sed -n '/class TestClientTimeoutConfiguration:/,$p'
echo '=== All PR changes to timeout tests ==='
base="$(git merge-base origin/main "$target")"
git diff --unified=40 "${base}...${target}" -- "$test_file"
echo '=== Boto dependency declarations ==='
fd -HI -t f 'pyproject.toml|requirements*.txt|Pipfile|poetry.lock|uv.lock|setup.py|setup.cfg' . -d 4 \
-x sh -c 'echo "=== $1 ==="; rg -n -i -C 3 "boto3|botocore" "$1" || true' sh {}
echo '=== Existing environment configuration conventions ==='
rg -n -C 3 --type py \
'total_max_attempts|max_attempts|_TIMEOUT|CONNECT_TIMEOUT|READ_TIMEOUT|math\.isfinite' \
src tests 2>/dev/null || trueLength of output: 62213
The timeout finding is addressed. Both clients now receive a bounded
No additional blocking defect found. I accept the timeout fix and the tracked scope split. [resolve] ✏️ Learnings added
You are interacting with an AI system. |
|
@coderabbitai Both inline comments posted at
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
repo='groupthinking/EventRelay'
commit='b5721eb4f'
provider='src/youtube_extension/integrations/cloud_ai/providers/aws_rekognition.py'
echo '=== Exact commit ==='
git rev-parse "$commit"
git show -s --format='commit=%H%nsubject=%s' "$commit"
echo '=== Client construction and timeout configuration at exact commit ==='
git show "${commit}:${provider}" | nl -ba | sed -n '100,220p'
git show "${commit}:${provider}" | rg -n -C 4 \
'def _timeout_config_kwargs|connect_timeout|read_timeout|max_attempts|mode|BotoConfig|session\.client|config=client_config'
echo '=== Local read at exact commit ==='
git show "${commit}:${provider}" | rg -n -C 5 \
'def _read_file_bytes|open\(image_url|_prepare_image_input'
echo '=== Equivalent local read on origin/main ==='
git show "origin/main:${provider}" | rg -n -C 5 \
'open\(image_url|_prepare_image_input' || true
echo '=== Follow-up issue ==='
gh issue view 1209 --repo "$repo" --json number,state,title,url,body
echo '=== PR review threads and bot inline comments ==='
gh api "repos/${repo}/pulls/1205/comments?per_page=100" \
--jq '.[] | select(.user.login | test("coderabbit"; "i")) | {id, path, line, original_line, commit_id, body, html_url}' || true
echo '=== PR review states ==='
gh pr view 1205 --repo "$repo" --json headRefOid,reviews \
--jq '{headRefOid, reviews: [.reviews[] | {author: .author.login, state, commit: .commit.oid, body}]}'Length of output: 2039 🏁 Script executed: #!/bin/bash
set -u
repo='groupthinking/EventRelay'
commit='b5721eb4f'
provider='src/youtube_extension/integrations/cloud_ai/providers/aws_rekognition.py'
echo '=== Exact commit ==='
git rev-parse "$commit"
git show -s --format='commit=%H%nsubject=%s' "$commit"
echo '=== Client construction and timeout configuration at exact commit ==='
git show "${commit}:${provider}" | sed -n '100,220p'
git show "${commit}:${provider}" | rg -n -C 4 \
'def _timeout_config_kwargs|connect_timeout|read_timeout|max_attempts|mode|BotoConfig|session\.client|config=client_config' || true
echo '=== Local read at exact commit ==='
git show "${commit}:${provider}" | rg -n -C 5 \
'def _read_file_bytes|open\(image_url|_prepare_image_input' || true
echo '=== Equivalent local read on origin/main ==='
git show "origin/main:${provider}" | rg -n -C 5 \
'open\(image_url|_prepare_image_input' || true
echo '=== Follow-up issue ==='
gh issue view 1209 --repo "$repo" --json number,state,title,url,body || true
echo '=== CodeRabbit inline review comments ==='
gh api "repos/${repo}/pulls/1205/comments?per_page=100" \
--jq '.[] | select(.user.login | test("coderabbit"; "i")) | {id, path, line, original_line, commit_id, body, html_url}' || true
echo '=== PR head and review states ==='
gh pr view 1205 --repo "$repo" --json headRefOid,reviews \
--jq '{headRefOid, reviews: [.reviews[] | {author: .author.login, state, commit: .commit.oid, body}]}' || trueLength of output: 64054 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo_owner='groupthinking'
repo_name='EventRelay'
pr_number='1205'
target_ids='3696898103 3696898106'
threads_json="$(
gh api graphql \
-F owner="$repo_owner" \
-F name="$repo_name" \
-F number="$pr_number" \
-f query='
query($owner: String!, $name: String!, $number: Int!) {
repository(owner: $owner, name: $name) {
pullRequest(number: $number) {
reviewThreads(first: 100) {
nodes {
id
isResolved
comments(first: 20) {
nodes {
databaseId
author { login }
path
line
body
}
}
}
}
}
}
}'
)"
for comment_id in $target_ids; do
thread_id="$(
printf '%s' "$threads_json" |
jq -r --argjson comment_id "$comment_id" '
.data.repository.pullRequest.reviewThreads.nodes[]
| select(any(.comments.nodes[]; .databaseId == $comment_id))
| .id
'
)"
if [ -z "$thread_id" ] || [ "$thread_id" = "null" ]; then
echo "ERROR: no review thread found for comment ${comment_id}" >&2
exit 1
fi
is_resolved="$(
printf '%s' "$threads_json" |
jq -r --arg thread_id "$thread_id" '
.data.repository.pullRequest.reviewThreads.nodes[]
| select(.id == $thread_id)
| .isResolved
'
)"
if [ "$is_resolved" = "true" ]; then
echo "already_resolved comment=${comment_id} thread=${thread_id}"
continue
fi
gh api graphql \
-F threadId="$thread_id" \
-f query='
mutation($threadId: ID!) {
resolveReviewThread(input: {threadId: $threadId}) {
thread { id isResolved }
}
}' \
--jq '.data.resolveReviewThread.thread | "resolved thread=\(.id) isResolved=\(.isResolved)"'
doneLength of output: 2038
I resolved only those two threads. I did not resolve the separate outside-diff finding. 🐇 You are interacting with an AI system. |
Remediation runbook — publish gate reached (awaiting human merge approval)Ran the per-PR loop after the Green:
Red — but non-blocking (
Publish gate is human-by-default. No Generated by Claude Code |
…1233) * perf: read local image bytes off the event loop in vision providers Azure `_prepare_image_input` and Google `analyze_image` read local image files with a synchronous `open().read()` inside `async def`, blocking the event loop for the duration of the disk read. Every other coroutine on the loop stalls until the read completes. Both now delegate to a module-level `_read_file_bytes` helper via `asyncio.to_thread`, matching the fix already merged for the AWS Rekognition sibling in #1205. This completes that cross-provider work so all three providers share one contract. The URL branches are untouched: Azure returns None so the SDK fetches the URL itself, and Google still sets `image.source.image_uri`. Also drops three now-redundant function-local `import asyncio` statements in azure_vision.py, made dead by the new module-level import. The lazy Azure SDK imports beside them are left in place. Regression tests assert off-loop execution by thread identity rather than wall-clock timing, which is flaky under CI load. Both new tests fail against the unpatched sources with "read on the event loop thread". Refs #1232 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * test(vision): record read-thread, not open-thread, in off-loop guards Copilot review on #1233 flagged that _ThreadRecordingOpen recorded the thread that called open(), not the thread that performed handle.read(). A regression offloading only open() while reading bytes back on the event loop would still pass, so the test did not prove #1232's required property. Wrap the returned handle in _ThreadRecordingHandle and record the calling thread on read() instead. Behaviour-preservation tests (URL branch, missing file) are unchanged; all off-loop guards still pass, and the recorder now fails on an open-offloaded/read-on-loop regression. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A6r6TyHudJkb6mn4HUNF5Y * test(google): pin missing-file CloudAIError wrapper contract CodeRabbit review on #1233 flagged that TestGoogleCloudImageReadOffEventLoop had no missing-file test, while GoogleCloudAI.analyze_image catches FileNotFoundError in its broad `except Exception` and re-raises CloudAIError — unlike Azure's private _prepare_image_input, which propagates FileNotFoundError. Add a Google test asserting the CloudAIError wrapper so moving the read off the event loop cannot silently change how a missing local image is reported. Closes the coverage gap; documents that the two providers differ at the tested surface (public analyze_image vs private helper). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A6r6TyHudJkb6mn4HUNF5Y * test(google): assert FileNotFoundError survives in the exception chain Strengthen the wrapper-contract guard added in e0961b2: asserting only pytest.raises(CloudAIError) would still pass if the underlying cause were swallowed or the message went generic. Also assert the original FileNotFoundError is preserved on __context__ (the provider re-raises without 'from e', so chaining is implicit) and that the path error text reaches the caller. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
Head:
b5721eb4fCanonical issue
Closes #1204
Outcome
boto3 is a synchronous SDK. Every Rekognition call sat directly inside an
async def, so each blocked the event loop for a whole network round-trip. All 14 now dispatch throughawait asyncio.to_thread(...)._wait_for_job_completion's poll loop (up to 120 blocking calls per analysis)_prepare_image_inputoff the loophttp(s)branch (alreadyhttpx.AsyncClient)detect_*callsThe win is loop availability, not latency. While one request waits on Rekognition, other requests, WebSocket frames and health checks now continue to be served.
Scope
src/youtube_extension/integrations/cloud_ai/providers/aws_rekognition.pyonly.await asyncio.to_thread(...)(asyncio.to_threadforwards**kwargs, so keyword arguments are unchanged)._read_file_bytes(path) -> byteshelper;_prepare_image_inputnow doesawait asyncio.to_thread(_read_file_bytes, image_url).Design notes
Why
to_threadand notgather. The fourdetect_*calls inanalyze_imagecould run concurrently, but that changes AWS request-rate behaviour and per-call error attribution. This PR deliberately keeps them sequential and fixes only the loop-blocking defect. Parallelising them is a separate, arguable change.Cancellation / use-after-close.
await asyncio.to_thread(...)is not cancellation-safe when a caller tears the client down in afinally— that was a real regression in #1190. HereAWSRekognition.cleanup()only setsself._rekognition_client = None(its own comment: "boto3 clients don't require explicit cleanup"); it closes no transport. A thread already holds the bound method, so an in-flight call completes normally. No shield is warranted — adding one would be unfalsifiable ceremony.Serialisation. These calls share no local mutable resource, so moving them off-loop removes no implicit ordering guarantee (contrast #1194, which needed a lock).
Risk
Low. Behaviour-preserving; all 89 pre-existing tests pass unmodified. The residual risk is thread-pool saturation under very high concurrency — bounded by the default executor, same as the already-merged #1190/#1196.
Verification
Non-vacuity (both dimensions reverted simultaneously): boto3 calls put back on the loop and
_read_file_bytescalled directly →Exactly the 5 heartbeat tests failed. The 6th new test is a guard (
test_local_image_bytes_are_read_correctly) and correctly still passed, as did all 89 pre-existing tests — confirming the mutation was behavioural, not structural.Each new test runs a heartbeat task alongside the work; blocking I/O on the loop yields 0 ticks.
Review round 2 — replaced a load-sensitive assertion
CI surfaced a single failure at
792604bd5:TestRekognitionDoesNotBlockEventLoop::test_local_image_read_does_not_stall_the_event_loop.This was the exact risk I flagged as challenge 4 in the review request. Unlike the four
boto3 tests — which drive a controllable 0.12s mock and passed on CI — the local file read
is a few microseconds of real work, so "did the event loop tick while it ran" measures
scheduler behaviour under load rather than the property I care about.
Replaced it with a direct assertion of the property: record
threading.get_ident()inside_read_file_bytesand require it to differ from the thread running the event loop. That isprecisely what "dispatched off the loop" means, requires no sleeps, and cannot flake under
runner contention.
Non-vacuity re-proven: reverting line 402 to call
_read_file_bytes(image_url)directlyinstead of via
asyncio.to_threadproduces exactly one targeted failure. The other fivetests in the class are unchanged and still pass. Suite: 95 passed.
Review round 3 — the deterministic test was patching the wrong namespace
Round 2's thread-identity test passed locally (including a full 7,731-test
single-process
tests/unitrun) but failed on CI withAssertionError: _read_file_bytes was never called— while the resultassertion immediately above it passed. That combination is diagnostic: the
real helper ran and returned the right bytes, but the recording wrapper was
never invoked. The module object returned by a fresh
importinside the testwas not the namespace
_prepare_image_inputresolves names from.The wider unit suite makes that reachable.
tests/unit/test_core_mcp_registry.pyre-registers canonical dotted names (
sys.modules[canonical] = mod) andtests/unit/test_v1_router_extended.pyinstallsMagicMock()underyoutube_extension.*names. Either leaves a class imported earlier holding__globals__pointing at a different dict than a later import returns, sopatch.object(<freshly imported module>, ...)silently patches nothing.Fix: patch
type(provider)._prepare_image_input.__globals__viapatch.dict.That is by definition the namespace the call site looks the name up in, so it
cannot drift no matter what sibling modules do to
sys.modules, and it needsno import at all.
The same round also parametrized the off-loop coverage so that every
converted SDK operation is asserted individually rather than in batches.
Non-vacuity (re-proved for the expanded suite)
Reverting all 15
await asyncio.to_thread(...)sites to direct callsproduces exactly 15 targeted failures, 90 passed — a 1:1 mapping of
converted call site to failing assertion. Restoring the source returns the
file to 105 passed.
test_aws_rekognition_provider.pytests/unit, one processorigin/mainAttribution
Commits
2c0fec7f7andaf04aedb5were pushed to this branch by anotheragent while I was preparing an equivalent fix. Its diagnosis matched mine, and
its
patch.dict(method_globals, ...)form is strictly better than thehand-rolled
try/finallyI had written — it restores on exception and isidiomatic. I adopted its commits rather than force-pushing over them, then
independently re-ran and re-proved the suite above.
Production evidence
...cloud_ai.providers.aws_rekognitionis in the transitive import closure of the primary production entrypointyoutube_extension.main:app(rootDockerfile:93), viamain.py:171→backend/cloud_ai_routes.py:89,141→integrations/cloud_ai/integrator.py:297-298.Agent handoff
Review focus: the cancellation argument above, and whether keeping the four
detect_*calls sequential is the right call.Review round 4 — bounding the work I moved into the shared thread pool
CodeRabbit raised two
Majorfindings ataf04aedb5. They were judged separately.Finding 2 — no botocore timeouts (
aws_rekognition.py:115) — ACCEPTED, fixed inb5721eb4fThis one is a direct consequence of this PR and was fixed here.
initialize()built both clients withsession.client('rekognition')/session.client('s3')and no
botocore.config.Config. On its own that is merely untidy. Combined with this PR itis a real hazard: the 14 SDK calls now run via
asyncio.to_thread, i.e. on the process-widedefault executor. botocore's defaults leave a request effectively unbounded, so a stalled AWS
call no longer blocks the event loop (the old, obvious failure) — it silently pins one of that
pool's limited worker threads forever. That pool is shared with the metrics persistence
(#1194), sqlite access (#1196) and result writes (#1203) already merged onto it, and
_wait_for_job_completioncan issue up to 120 such calls per job. This is the samepool-exhaustion class of regression caught on #1152.
Fix: both clients are constructed with an explicit
Config(connect_timeout, read_timeout, retries={'max_attempts', 'mode': 'standard'}). Values are overridable viaAWS_REKOGNITION_CONNECT_TIMEOUT/_READ_TIMEOUT/_MAX_ATTEMPTS, validated withmath.isfiniteand raising rather than clamping, soinf,nan,0and negatives arerejected outright.
Parsing is hoisted above
initialize()'stry, because that method ends in a catch-allexcept Exception -> CloudAIErrorwhich would otherwise bury a preciseConfigurationErrorbehind a generic init failure.
Finding 1 — local path traversal (
aws_rekognition.py:37) — REJECTED as out of scope, tracked in #1209The finding is correct on the merits, but it is pre-existing and unchanged by this PR.
origin/mainalready read the value verbatim at line 373:This PR moved that read onto a worker thread. The set of readable paths is byte-for-byte
identical before and after — no widening. Fixing it properly means choosing and enforcing a
media-root policy, handling symlink escape as well as lexical
../, and deciding whether thelocal branch should exist in production at all. That is a behavioural security change that
deserves its own PR and its own tests, not a rider on a performance PR.
Filed as #1209 with full acceptance criteria. This mirrors the scope objection CodeRabbit
itself raised on #1152, where an out-of-scope guard was reverted and tracked as #1162.
Verification of round 4
config=dropped from both clientsBreakdown of the 18: 1 client-config assertion, 1 env-override assertion, 12 timeout-rejection
cases (2 vars x 6 bad values), 4 max-attempts rejection cases. The three "blank value falls back
to default" cases and the "defaults are finite" case correctly still pass — they are guards that
survive this mutation by design.
Suite: 127 passed (105 pre-existing, unmodified except for adding a symmetric
"botocore.config"entry to the three existingpatch.dict("sys.modules", ...)stubs, whichalready stubbed
"botocore.exceptions"; plus 22 new). ruff parity withorigin/mainunchanged(8 = 8).