perf(cloud-ai): analyse each batch concurrently in process_batch_videos - #1188
Conversation
The batch loop sliced video_urls into batch_size chunks but then awaited ai.analyze_video() for each video in turn, so batch_size controlled nothing except how often the inter-batch pause fired - wall-clock cost stayed the full sum of every per-video analysis. Analyse each batch with asyncio.gather(..., return_exceptions=True) so batch_size becomes the real bound on concurrent calls to the shared upstream AI providers. isinstance(result, Exception) preserves the previous except-Exception-and-continue semantics exactly, so one failing video is logged and skipped without aborting its batch. Also hoists the loop-local 'import asyncio' to module scope. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Repository UI (inherited), Organization UI (inherited) Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
📜 Recent review details⏰ Context from checks skipped due to timeout. (6)
|
| Layer / File(s) | Summary |
|---|---|
Concurrent batch execution src/youtube_extension/backend/cloud_ai_routes.py |
The module imports asyncio. Video analyses within each batch run concurrently. Ordinary per-video exceptions are logged and skipped, cancellation-like exceptions are re-raised, successful results are formatted, and the inter-batch pause remains. |
Estimated code review effort: 2 (Simple) | ~15 minutes
Suggested labels: copilot-rabbit
Suggested reviewers: claude
Poem
Videos gather in flight,
Failures fade from sight,
Cancellations break through,
Results return anew,
Then batches pause just right.
🚥 Pre-merge checks | ✅ 4 | ❌ 3
❌ Failed checks (3 inconclusive)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Linked Issues check | ❓ Inconclusive | The implementation matches issue #1187, but the required regression tests cannot be verified because the test file is excluded by !tests/**. |
Include reviewable evidence for tests/unit/test_cloud_ai_routes_batch.py or remove the !tests/** exclusion so the regression tests can be verified. |
| Enforce Copilot Verification | ❓ Inconclusive | Evidence collection is still in progress. | Inspect the pull request review records and verify an explicit approval from GitHub Copilot. |
| Require Ai Unit Tests | ❓ Inconclusive | The repository confirms committed batch tests, but it contains no pull-request label state; external PR metadata is required to verify copilot-rabbit. |
Provide the pull request URL or accessible GitHub metadata showing the current PR labels. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title clearly describes the primary change: concurrent per-batch analysis in cloud AI processing. |
| Description check | ✅ Passed | The description covers the change, scope, risks, verification, production reachability, and handoff with sufficient detail. |
| Out of Scope Changes check | ✅ Passed | The reviewed changes support issue #1187 and contain no unrelated production code changes. |
| Docstring Coverage | ✅ Passed | Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. |
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
- Create stacked PR
- Commit on current branch
📝 Generate docstrings
- Create stacked PR
- Commit on current branch
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
perf/batch-video-analyze
✨ Simplify code
- Create PR with simplified code
- Commit simplified code in branch
perf/batch-video-analyze
Warning
Review ran into problems
🔥 Problems
These 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 groupthinking/uvai-skills.
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 help to get the list of available commands.
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 review Please review with particular attention to these five points — I would rather have them challenged now than after merge:
|
Agent Completion Truth Gate: NOT_APPLICABLEEvidence agrees. Machine-readable verdict{
"details": {},
"reasons": [],
"verdict": "not_applicable"
} |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== Changed files =='
git diff --name-only origin/main...HEAD
echo '== Route and integrator outlines =='
ast-grep outline src/youtube_extension/backend/cloud_ai_routes.py --items all --match 'process_batch_videos|analyze_video' --view expanded || true
fd -i 'cloud_ai*' src tests | sort
echo '== Batch implementation and endpoint context =='
rg -n -C 12 'async def process_batch_videos|process_batch_videos\s*\(|analyze/batch|background_tasks\.add_task' \
src/youtube_extension/backend/cloud_ai_routes.py
echo '== Integrator definitions, fallback, provider calls, mutable instance state, and rate-limit handling =='
rg -n -C 8 --glob '*.py' \
'class CloudAIIntegrator|async def analyze_video|def analyze_video|use_fallback|fallback|RateLimitError|rate.limit|aiohttp\.ClientSession|self\.[A-Za-z_]+\s*=' \
src/youtube_extension
echo '== New batch test outline and relevant test body =='
ast-grep outline tests/unit/test_cloud_ai_routes_batch.py --items all --view expanded || true
cat -n tests/unit/test_cloud_ai_routes_batch.py
echo '== Exact PR diff =='
git diff --unified=80 origin/main...HEAD -- \
src/youtube_extension/backend/cloud_ai_routes.py \
tests/unit/test_cloud_ai_routes_batch.pyLength of output: 201880 🏁 Script executed: #!/bin/bash
set -u
echo '== Changed paths =='
git diff --name-only origin/main...HEAD 2>&1 | sed -n '1,100p'
echo '== Candidate route, integrator, and batch-test paths =='
fd -t f -i 'cloud_ai_routes.py' . | sort
fd -t f -i '*cloud*ai*integrator*.py' src tests | sort
fd -t f -i '*cloud*ai*route*batch*.py' tests . | sort
echo '== PR diff summary =='
git diff --stat origin/main...HEAD 2>&1 | sed -n '1,120p'
echo '== Only relevant diff lines =='
git diff --unified=35 origin/main...HEAD -- \
':(glob)**/cloud_ai_routes.py' \
':(glob)**/test_cloud_ai_routes_batch.py' 2>&1 | sed -n '1,800p'
route="$(fd -t f -i 'cloud_ai_routes.py' src | head -n 1)"
if [ -n "$route" ]; then
echo "== $route: batch code =="
rg -n -C 20 'async def process_batch_videos|process_batch_videos\s*\(|asyncio\.gather|use_fallback' "$route" | sed -n '1,500p'
fi
echo '== CloudAIIntegrator candidates and definitions =='
rg -n -l --glob '*.py' 'class CloudAIIntegrator\b' src tests 2>/dev/null | while IFS= read -r f; do
echo "== $f =="
rg -n -C 30 'class CloudAIIntegrator\b|async def analyze_video\b|def analyze_video\b|use_fallback|asyncio\.gather|self\.[A-Za-z_]+\s*=' "$f" | sed -n '1,1000p'
done
test="$(fd -t f -i 'test_cloud_ai_routes_batch.py' . | head -n 1)"
if [ -n "$test" ]; then
echo "== $test =="
cat -n "$test" | sed -n '1,700p'
fiLength of output: 27764 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== Python/runtime metadata =='
for f in pyproject.toml setup.cfg setup.py requirements.txt requirements-dev.txt; do
if [ -f "$f" ]; then
echo "== $f =="
rg -n -C 3 'python|aiohttp|google|boto|azure' "$f" || true
fi
done
echo '== Integrator initialization methods and full analyze_video body =='
sed -n '42,135p' src/youtube_extension/integrations/cloud_ai/integrator.py
rg -n -C 25 'async def _initialize_(google_cloud|aws_rekognition|azure_vision)' \
src/youtube_extension/integrations/cloud_ai/integrator.py
echo '== Cloud AI provider source files =='
fd -t f -e py . src/youtube_extension/integrations/cloud_ai/providers | sort
for f in $(fd -t f -e py . src/youtube_extension/integrations/cloud_ai/providers | sort); do
echo "== $f: analyze_video and mutable state =="
rg -n -C 20 \
'async def analyze_video\b|async def initialize\b|async def cleanup\b|self\.[A-Za-z_]+\s*=|asyncio\.gather|create_task|RateLimitError|retry|sleep\(' \
"$f" | sed -n '1,700p'
done
echo '== Existing tests for integrator cancellation, fallback, and rate limits =='
rg -n -C 8 --glob '*.py' \
'CancelledError|return_exceptions|RateLimitError|retry_after|use_fallback|analyze_video' \
src/youtube_extension/integrations/cloud_ai tests/unit 2>/dev/null | sed -n '1,1000p'
echo '== Batch test path and cancellation/rate-limit coverage =='
git show HEAD:tests/unit/test_cloud_ai_routes_batch.py | nl -ba | sed -n '1,220p'Length of output: 14804 🏁 Script executed: #!/bin/bash
set -u
for f in \
src/youtube_extension/integrations/cloud_ai/providers/google_cloud.py \
src/youtube_extension/integrations/cloud_ai/providers/aws_rekognition.py \
src/youtube_extension/integrations/cloud_ai/providers/azure_vision.py; do
echo "== $f: outline =="
ast-grep outline "$f" --items all --view expanded 2>&1 | sed -n '1,260p'
echo "== $f: complete bounded source =="
nl -ba "$f" | sed -n '1,520p'
done
echo '== All direct provider calls and per-call task fan-out in provider modules =='
rg -n -C 8 --glob '*.py' \
'\.(annotate_video|annotate_image|start_label_detection|start_face_detection|start_person_tracking|start_segment_detection|analyze_video|analyze_image)\s*\(|asyncio\.(gather|create_task)|run_in_executor|to_thread' \
src/youtube_extension/integrations/cloud_ai/providers 2>&1 | sed -n '1,900p'
echo '== Rate-limit handling and retry loops in the Cloud AI path =='
rg -n -C 10 --glob '*.py' \
'RateLimitError|retry_after|for .*range.*retry|backoff|await asyncio\.sleep' \
src/youtube_extension/integrations/cloud_ai 2>&1 | sed -n '1,900p'
echo '== Test names and async test configuration =='
git ls-tree -r --name-only HEAD | rg '(^|/)test_cloud_ai_(routes_batch|integrator|exceptions|config)\.py$' || true
git show HEAD:tests/unit/test_cloud_ai_routes_batch.py | nl -ba | sed -n '1,180p'Length of output: 49397 🏁 Script executed: #!/bin/bash
set -u
echo '== Formatter contract and batch endpoint =='
rg -n -C 35 \
'^def format_analysis_result\b|^async def format_analysis_result\b|`@router`\.post\("/analyze/batch"|background_tasks\.add_task|process_batch_videos' \
src/youtube_extension/backend/cloud_ai_routes.py 2>&1 | sed -n '1,700p'
echo '== Attributes read from a VideoAnalysisResult by the formatter =='
sed -n '1,260p' src/youtube_extension/backend/cloud_ai_routes.py
echo '== Repository references to child-task cancellation handling =='
rg -n -C 8 --glob '*.py' \
'gather\(.+return_exceptions=True|isinstance\(.*BaseException|except asyncio\.CancelledError|raise.*CancelledError' \
src tests 2>&1 | sed -n '1,800p'Length of output: 83487
Blocking — cancellation is not fully preserved. The old sequential loop would propagate My assessment of the five points follows.
🐇 You are interacting with an AI system. |
There was a problem hiding this comment.
Pull request overview
Improves batch video-analysis latency by running each batch concurrently while preserving ordering and failure isolation.
Changes:
- Uses
asyncio.gatherfor bounded concurrent analysis. - Adds tests for concurrency, batching, failures, pauses, and empty input.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/youtube_extension/backend/cloud_ai_routes.py |
Adds concurrent batch processing. |
tests/unit/test_cloud_ai_routes_batch.py |
Adds batch-processing regression tests. |
groupthinking
left a comment
There was a problem hiding this comment.
Independent review — verified the blocking finding and one accuracy caveat at head 757e8475.
I checked the two material claims against the source rather than echoing the bot.
1. Correctness — child CancelledError is mishandled (confirms CodeRabbit; contradicts this PR's own rationale).
Verified the mechanism, not just the conclusion: asyncio.gather(..., return_exceptions=True) appends a cancelled child's CancelledError as a result element — CPython's gather._done_callback builds the results list with a CancelledError for any fut.cancelled(), it does not re-raise it. At cloud_ai_routes.py:404, isinstance(result, Exception) is False for CancelledError (it derives from BaseException), so control falls through to format_analysis_result(result) → AttributeError (no .video_id) → swallowed by the function-level except Exception at :418.
This is the exact opposite of the PR body's stated design ("cancellation still propagates rather than being silently swallowed as a failed video"). The old await ai.analyze_video() under except Exception: did let a CancelledError propagate; the new code does not. Real-world trigger probability is low here (nothing external holds the internal gather children to cancel them individually), but since the PR documents the opposite guarantee, code and doc need reconciling either way. Minimal fix:
for video_url, result in zip(batch, batch_results, strict=True):
if isinstance(result, BaseException) and not isinstance(result, Exception):
raise result # CancelledError / KeyboardInterrupt / SystemExit — propagate, as before
if isinstance(result, Exception):
logger.error(f"Failed to analyze video {video_url}: {result}")
continue
results.append(format_analysis_result(result))For the test (your point 5): a fake analyze_video raising asyncio.CancelledError must assert that process_batch_videos re-raises it — note that with format_analysis_result mocked to identity the current code would neither raise nor crash, so an assertion on "formatter error" would be a false pass; assert propagation.
2. Accuracy — the "~slowest single analysis" wall-clock claim holds for only 2 of the 3 providers.
The before/after table promises batch wall-clock drops to the slowest single analysis. Verified this is provider-dependent:
- Google (
providers/google_cloud.py): native async client (await self._video_client.annotate_video,await asyncio.wait_for(operation.result(), ...)) → genuinely concurrent undergather. ✓ - Azure (
providers/azure_vision.py:273): offloads blocking SDK calls viaasyncio.to_thread→ genuinely concurrent. ✓ - AWS Rekognition (
providers/aws_rekognition.py): synchronous boto3 calls made directly on the event loop —start_label_detection/get_*_detectionat lines 281–335, noto_thread/run_in_executor. These block the loop, sogathercannot overlap the RPCs; only theawait asyncio.sleep(poll_interval)at :345 interleaves. For an AWS-served batch the wall-clock stays ~sum of the synchronous RPC latencies, and the batch now holds the loop across those calls.
This doesn't make the change unsafe — failure isolation, ordering, and the inter-batch pause are all preserved — but the perf outcome is overstated for the AWS path, and the new tests (which mock analyze_video) prove route-level coroutine scheduling, not real provider-I/O concurrency. Suggest either scoping the claim to the async providers in the PR body, or wrapping the sync boto3 calls in to_thread as a separate change.
On your five questions: answers to 1, 4, 5 match my read — batch_size bounds concurrency per process_batch_videos invocation only, not globally across simultaneous /analyze/batch requests (each spins its own integrator); RateLimitError retry is correctly out of scope; and the two peak-concurrency assertions do reject the sequential implementation. Item 2 is the confirmed bug above. Item 3 — no unsafe per-video shared state on CloudAIIntegrator, but see the AWS blocking caveat.
Net: one correctness fix (item 1) to match the documented intent, and one honest scoping of the perf claim (item 2). Both small; neither is a redesign.
Generated by Claude Code
`asyncio.gather(..., return_exceptions=True)` captures a child's CancelledError as a *value*. Because it derives from BaseException and not Exception, the `isinstance(result, Exception)` filter let it fall through to `format_analysis_result`, which raised AttributeError that the outer `except Exception` then swallowed -- silently skipping every remaining batch. Re-raise BaseException-but-not-Exception results before the per-video failure handling, restoring the propagation semantics of the original sequential `await`/`except Exception` loop. Reported by @Copilot in review of #1188. Adds 2 regression tests that fail with "DID NOT RAISE CancelledError" without this change. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
@copilot — this finding is correct, and it was worse than a swallowed cancellation. Thank you. You are right that Fixed at # `return_exceptions=True` also captures BaseException-only failures
# (most notably CancelledError) as *values*, so an
# `isinstance(..., Exception)` test alone would let a cancelled child
# fall through to format_analysis_result. Re-raise those to preserve
# the previous `await`/`except Exception` semantics.
cancellations = [
result
for result in batch_results
if isinstance(result, BaseException) and not isinstance(result, Exception)
]
if cancellations:
raise cancellations[0]On ordering — you asked for the re-raise to come before the per-video failure logging, and Regression tests — two added, both proven non-vacuous by removing only the re-raise block and The first test uses 4 URLs at Verification at I have also corrected the PR body, which previously implied @coderabbitai review |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== Current PR head and relevant changed files =='
git rev-parse --short HEAD
git diff --name-only origin/main...HEAD
git diff --unified=70 origin/main...HEAD -- \
src/youtube_extension/backend/cloud_ai_routes.py \
tests/unit/test_cloud_ai_routes_batch.py
echo '== Batch function structure and implementation =='
ast-grep outline src/youtube_extension/backend/cloud_ai_routes.py \
--items all --match 'process_batch_videos|format_analysis_result' --view expanded
rg -n -C 45 \
'^async def process_batch_videos\b|asyncio\.gather|batch_results|cancellations|format_analysis_result|await asyncio\.sleep' \
src/youtube_extension/backend/cloud_ai_routes.py
echo '== Integrator exception behavior =='
integrator="$(fd -t f -i 'integrator.py' src/youtube_extension/integrations/cloud_ai | head -n 1)"
printf 'Integrator: %s\n' "$integrator"
rg -n -C 35 \
'^class CloudAIIntegrator\b|^ async def analyze_video\b|except (Exception|BaseException|asyncio\.CancelledError)|use_fallback' \
"$integrator"
echo '== Batch test structure and bodies =='
ast-grep outline tests/unit/test_cloud_ai_routes_batch.py --items all --view expanded
cat -n tests/unit/test_cloud_ai_routes_batch.py
echo '== Existing cancellation handling conventions =='
rg -n -C 5 --glob '*.py' \
'gather\([^)]*return_exceptions=True|isinstance\([^)]*BaseException|except asyncio\.CancelledError' \
src testsLength of output: 48049 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== Committed cancellation tests =='
git show HEAD:tests/unit/test_cloud_ai_routes_batch.py | nl -ba | sed -n '115,230p'
echo '== Pytest async configuration =='
for f in pyproject.toml pytest.ini setup.cfg tox.ini; do
if git cat-file -e "HEAD:$f" 2>/dev/null; then
echo "-- $f --"
git show "HEAD:$f" | rg -n -C 3 'asyncio_mode|pytest|asyncio' || true
fi
done
echo '== Standard-library gather behavior for child cancellation =='
python - <<'PY'
import asyncio
async def cancelled():
raise asyncio.CancelledError("child cancellation")
async def main():
result, = await asyncio.gather(cancelled(), return_exceptions=True)
print(type(result).__module__ + "." + type(result).__qualname__)
print("is BaseException:", isinstance(result, BaseException))
print("is Exception:", isinstance(result, Exception))
asyncio.run(main())
PY
echo '== Verify current source catches only Exception at the outer task boundary =='
sed -n '366,438p' src/youtube_extension/backend/cloud_ai_routes.py | nl -baLength of output: 1438 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== Committed cancellation tests =='
git show HEAD:tests/unit/test_cloud_ai_routes_batch.py | sed -n '115,230p'
echo '== Pytest async configuration =='
for f in pyproject.toml pytest.ini setup.cfg tox.ini; do
if git cat-file -e "HEAD:$f" 2>/dev/null; then
echo "-- $f --"
git show "HEAD:$f" | rg -n -C 3 'asyncio_mode|pytest|asyncio' || true
fi
done
echo '== Current cancellation and outer-handler source =='
sed -n '387,436p' src/youtube_extension/backend/cloud_ai_routes.py
echo '== Test file identity in the commit versus worktree =='
git ls-tree -r --name-only HEAD | rg '^tests/unit/test_cloud_ai_routes_batch\.py$'
test -e tests/unit/test_cloud_ai_routes_batch.py && echo 'worktree test: present' || echo 'worktree test: absent'Length of output: 9241
The two new tests cover the required cases:
I found no additional production-impacting issue in this change. I could not independently run the new test file because the sandbox worktree does not contain it, although the file is present in commit 🐇 ✅ ✏️ Learnings added
If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! ✅ Action performedReview finished.
|
The fix in One minor note on the test for |
Canonical issue
Closes #1187
Outcome
process_batch_videosnow analyses each batch concurrently, sobatch_sizebecomes the real bound on in-flight calls to the shared upstream AI providers instead of a parameter that only set the cadence of asleep(1).analyze_videoper batchbatch_sizebatch_sizecontrolledWhat this does not change: the number of provider API calls, token spend, or quota consumed — it is strictly a latency change. One request per video, exactly as before.
Scope
src/youtube_extension/backend/cloud_ai_routes.pyfor video_url in batch:loop →asyncio.gather(..., return_exceptions=True)import asynciohoisted to module scopetests/unit/test_cloud_ai_routes_batch.py(new, 5 tests)Design notes
Why unbounded
gatherover the batch is the correct bound here.analyze_videofans out over a shared resource — oneCloudAIIntegratorHTTP client and a provider quota — so this genuinely needs a concurrency limit, unlike a WebSocket broadcast where each peer owns an independent send buffer. The limit already exists: it isbatch_size, supplied by the caller and already used to slice the work. Gathering within a batch and keeping batches strictly sequential means peak in-flight calls can never exceedbatch_size. Adding a second semaphore inside the batch would be redundant and would let the two bounds drift apart.test_batch_size_bounds_concurrencypins this contract.Why
isinstance(result, Exception)— and why that alone was WRONG (corrected at8725b51d). This note originally claimed the check was exact parity for theexcept Exception:clause it replaces, on the reasoning thatCancelledErrorderives fromBaseExceptionand so would still propagate. That was backwards.asyncio.gather(..., return_exceptions=True)captures a child'sCancelledErroras a value rather than raising it, soisinstance(result, Exception)returnedFalseand the error object fell through toformat_analysis_result— raising anAttributeErrorthat the outerexcept Exceptionswallowed, silently abandoning every remaining batch. Caught by @copilot in review. The code now collectsBaseException-but-not-Exceptionresults and re-raises the first one before the per-video failure handling, restoring the original propagation semantics; two regression tests cover it.zip(..., strict=True)is retained so a length mismatch is loud.Ordering.
asyncio.gatherreturns results in argument order, soresultsis appended in the same order the sequential loop produced.Risk
Low. Behaviour-preserving apart from concurrency: per-video failure isolation, result ordering, the inter-batch pause, and the log message on failure are all unchanged and pinned by tests. The function is a fire-and-forget
BackgroundTasksjob, so it is not on any request's critical path.Verification
At head
757e8475c:Non-vacuity — behavioural mutation. Module-level
import asynciokept, only the semantics reverted to the sequential loop:The other 3 tests pass under both implementations by design — they are regression guards for the behaviour this PR must preserve (failure isolation, pause cadence, empty input).
Production evidence
Dockerfile:93→youtube_extension.main:app→main.py:171importscloud_ai_routes→POST /api/v1/cloud-ai/analyze/batch(cloud_ai_routes.py:288) →background_tasks.add_task(process_batch_videos, ...)(:289).cloud_ai_routesis in the transitive import closure of the deployed application.Agent handoff
Reviewed by
@coderabbitai(see review request comment). Follow-ups, if any, will be filed as separate issues rather than expanding this PR's scope.