Refactor(#60): Gemini 분석기를 AWS Bedrock(Claude Haiku)로 교체 - #61
Conversation
pearseona
added
the
refactor
📝 WalkthroughWalkthroughThe PR replaces Gemini with a provider-neutral LLM layer backed by AWS Bedrock. It adds shared client contracts, structured analysis validation, hybrid routing updates, RabbitMQ compatibility fields, threshold-selection changes, configuration updates, and tests. ChangesProvider-neutral LLM migration
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant AnalysisService
participant HybridTextAnalyzer
participant LlmSmishingAnalyzer
participant BedrockLlmClient
AnalysisService->>HybridTextAnalyzer: analyze text with force_llm
HybridTextAnalyzer->>LlmSmishingAnalyzer: request LLM review
LlmSmishingAnalyzer->>BedrockLlmClient: generate prompt and messages
BedrockLlmClient-->>LlmSmishingAnalyzer: return LlmGeneration
LlmSmishingAnalyzer-->>HybridTextAnalyzer: return validated analysis
HybridTextAnalyzer-->>AnalysisService: return routed analysis result
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (8)
tests/infrastructure/rabbitmq/test_result_factory.py (1)
252-271: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winIsolate the legacy alias path.
Line 257 leaves
llm_available=Truefrom_result(). An incorrect mapper that derivesllmCalledfrom availability can pass this test without readinggemini_called. Setllm_availabletoFalsebefore creating the event.Proposed fix
assert result.text_analysis is not None result.text_analysis.pop("llm_called") + result.text_analysis["llm_available"] = False result.text_analysis["gemini_called"] = True🤖 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 `@tests/infrastructure/rabbitmq/test_result_factory.py` around lines 252 - 271, Update test_factory_reads_legacy_gemini_called_alias to set result.text_analysis["llm_available"] to False after removing llm_called and setting gemini_called. Keep the assertion focused on verifying that the factory reads the legacy gemini_called alias rather than deriving llmCalled from availability.tests/data_science/SMSModel/modeling/test_hybrid_thresholds.py (1)
100-100: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse provider-neutral terminology.
Line 100 says “Gemini 호출”. The test validates LLM routing. Rename the comment to use “LLM” terminology.
Proposed fix
- # Stacking만으로 완벽히 분류할 수 있으므로 Gemini 호출이 필요하지 않아야 함 + # Stacking만으로 완벽히 분류할 수 있으므로 LLM 호출이 필요하지 않아야 함🤖 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 `@tests/data_science/SMSModel/modeling/test_hybrid_thresholds.py` at line 100, Update the comment near the hybrid-threshold test to replace the provider-specific “Gemini 호출” wording with provider-neutral “LLM” terminology, while preserving its meaning that stacking alone should avoid an LLM call.app/core/config.py (2)
93-93: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFix the comment typo.
ReabbitMQshould readRabbitMQ.🤖 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 `@app/core/config.py` at line 93, Correct the comment typo by changing “ReabbitMQ” to “RabbitMQ” in the execution settings comment.
125-141: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFold the blank checks into
required_values.Lines 125-133 check
AWS_REGIONandBEDROCK_MODEL_IDfor blank values. Lines 135-137 add the same two fields torequired_values, and line 147 applies an equivalent blank check. Keep one mechanism so that all missing production settings report in a single aggregated error message.♻️ Proposed refactor
- if not self.AWS_REGION.strip(): - raise ValueError( - "AWS_REGION must not be blank" - ) - - if not self.BEDROCK_MODEL_ID.strip(): - raise ValueError( - "BEDROCK_MODEL_ID must not be blank" - ) - required_values = {🤖 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 `@app/core/config.py` around lines 125 - 141, Remove the separate blank-value checks for AWS_REGION and BEDROCK_MODEL_ID, and rely on the existing required_values validation to process them with the other production settings. Preserve the aggregated error reporting so all missing or blank settings are reported together.app/infrastructure/llm/__init__.py (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFix the docstring typo.
ingreastructureshould readinfrastructure.📝 Proposed fix
-"""Provider-neutral LLM ingreastructure.""" +"""Provider-neutral LLM infrastructure."""🤖 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 `@app/infrastructure/llm/__init__.py` at line 1, Correct the module docstring in app/infrastructure/llm/__init__.py by changing the misspelled “ingreastructure” to “infrastructure.”app/analysis/text/llm_analyzer.py (3)
216-216: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSuppress BLE001 instead of narrowing the catch.
Ruff flags the blind
except Exception. The catch is correct here, because the analyzer must degrade to a fallback result rather than fail the whole analysis. If Ruff gates CI, add an explicit suppression with the reason.🔧 Proposed fix
- except Exception as exception: + except Exception as exception: # noqa: BLE001 - degrade to a fallback result🤖 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 `@app/analysis/text/llm_analyzer.py` at line 216, Update the exception handler in the analyzer flow around `except Exception as exception` to retain the broad catch and add an explicit Ruff BLE001 suppression with a concise reason that the analyzer must return its fallback result instead of failing the overall analysis.Source: Linters/SAST tools
13-13: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winProvider-neutral modules import a provider-specific exception.
LlmProviderErroris defined inapp/infrastructure/llm/bedrock_client.py, so both consumers depend on the Bedrock implementation module to catch a normalized error. Define the exception inapp/infrastructure/llm/types.pynext toLlmGenerationandLlmClient, re-export it frombedrock_client.pyfor compatibility, then update both imports.
app/analysis/text/llm_analyzer.py#L13: importLlmProviderErrorfromapp.infrastructure.llm.types.app/chat/service.py#L9: importLlmProviderErrorfromapp.infrastructure.llm.types.🤖 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 `@app/analysis/text/llm_analyzer.py` at line 13, Move LlmProviderError into app/infrastructure/llm/types.py alongside LlmGeneration and LlmClient, and re-export it from bedrock_client.py to preserve compatibility. Update the imports in app/analysis/text/llm_analyzer.py:13 and app/chat/service.py:9 to use app.infrastructure.llm.types instead of the Bedrock module.
96-102: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the hardcoded Bedrock identifiers from the provider-neutral analyzer.
_failure_resultdefaultsproviderto"AWS_BEDROCK"andmodel_idtosettings.BEDROCK_MODEL_ID. Lines 146-147 repeat the same defaults. The module docstring declares the analyzer provider-neutral, so these values contradict it and report the wrong provider once a second provider exists.Report
Nonewhen the client is unknown, and let the caller treat a missing provider as unattributed.♻️ Proposed refactor
def _failure_result( error_code: str, *, - provider: str = "AWS_BEDROCK", + provider: str | None = None, model_id: str | None = None, ) -> dict[str, Any]: return { "is_mock": False, "provider": provider, - "model_id": model_id or settings.BEDROCK_MODEL_ID, + "model_id": model_id,- provider = getattr(llm_client, "provider", "AWS_BEDROCK") - model_id = getattr(llm_client, "model_id", settings.BEDROCK_MODEL_ID) + provider = getattr(llm_client, "provider", None) + model_id = getattr(llm_client, "model_id", None)Confirm that
app/infrastructure/rabbitmq/result_factory.pytolerates aNonellm_provider;TextAnalysisDetail.llmProvideris alreadystr | None.Also applies to: 146-147
🤖 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 `@app/analysis/text/llm_analyzer.py` around lines 96 - 102, Remove the AWS Bedrock defaults from the provider-neutral _failure_result function and its repeated caller values around the analysis failure path. Preserve explicit provider and model identifiers when available, but pass or return None for unknown clients so missing providers remain unattributed; verify result_factory handling continues to accept a None llm_provider.
🤖 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 `@app/analysis/hybrid_policy.py`:
- Around line 51-69: In app/analysis/hybrid_policy.py#L51-L69, validate
result["risk_score"] before accepting the stacking result: route to LLM_FALLBACK
when it is Boolean, non-integer, below 0, or above 100. In
app/analysis/text/hybrid_analyzer.py#L28-L45, update _stacking_to_public_result
to reject risk_score values below 0 or above 100 while preserving valid integer
scores.
In `@app/core/config.py`:
- Around line 32-34: Confirm the target account’s Bedrock inference profile with
aws bedrock list-inference-profiles --region us-east-1, then update the
BEDROCK_MODEL_ID default in app/core/config.py#L32-L34 to the required regional
profile identifier, using the us. prefix if applicable. Update the sample values
at .env.example#L6 and .env.prod.example#L6 to match the corrected identifier,
selecting the production prefix for its deployment geography.
- Around line 86-87: Update the Settings declarations by removing the duplicate
RABBITMQ_ANALYSIS_REQUEST_ROUTING_KEY entry and adding
RABBITMQ_ANALYSIS_FAILED_ROUTING_KEY with the intended failed-analysis routing
value, matching the setting consumed by publisher logic.
In `@app/infrastructure/llm/bedrock_client.py`:
- Around line 59-73: Update the Bedrock client’s __init__ and generate flow to
enforce an end-to-end deadline: compute and store self._total_timeout_seconds
from settings.LLM_TIMEOUT_SECONDS and settings.LLM_MAX_RETRIES, wrap the
asyncio.to_thread request in asyncio.wait_for using that budget, and map
asyncio.TimeoutError to LLM_TIMEOUT.
- Around line 160-194: Update BedrockClient._convert_messages to validate that
messages begin with user and strictly alternate between user and assistant
before constructing the converted list. Raise the existing request-validation
exception for invalid sequences so client errors do not reach Converse or become
provider errors, while preserving the current role and content validation.
In `@scripts/adversarial_test/mutations.py`:
- Around line 145-150: Update _extract_text to reject provider-neutral English
refusal phrases, including “I can't help with that,” alongside the existing
_REFUSAL_MARKERS before returning text. Preserve the current empty-response
validation and ensure refusal text is never accepted as a mutation.
In `@scripts/benchmark/measure_llm_timing.py`:
- Line 47: Reject responses where result["is_mock"] is true before recording
benchmark data: in scripts/benchmark/measure_llm_timing.py at lines 47-47,
record the returned model_id instead of settings.BEDROCK_MODEL_ID; in
scripts/benchmark/run_llm_track.py at lines 52-52, reject mock results before
appending rows and use the returned model_id; at lines 79-79, log the validated
model identifier or explicitly label the value as the configured model.
In `@tests/data_science/SMSModel/test_hybrid_threshold_selection.py`:
- Around line 38-58: Update test_rejects_cache_runtime_mismatch to derive each
mismatched value from the configured runtime rather than using fixed literals:
append "-mismatch" to the configured model ID for model_id and to the configured
region for region, while preserving the existing parametrized validation and
error assertions.
In `@tests/infrastructure/llm/test_bedrock_client.py`:
- Line 119: Update the pytest.raises assertion’s match argument to use a raw
regex literal, changing the anchored pattern in the relevant test to the
raw-string form while preserving the existing pattern and expected
LlmProviderError behavior.
---
Nitpick comments:
In `@app/analysis/text/llm_analyzer.py`:
- Line 216: Update the exception handler in the analyzer flow around `except
Exception as exception` to retain the broad catch and add an explicit Ruff
BLE001 suppression with a concise reason that the analyzer must return its
fallback result instead of failing the overall analysis.
- Line 13: Move LlmProviderError into app/infrastructure/llm/types.py alongside
LlmGeneration and LlmClient, and re-export it from bedrock_client.py to preserve
compatibility. Update the imports in app/analysis/text/llm_analyzer.py:13 and
app/chat/service.py:9 to use app.infrastructure.llm.types instead of the Bedrock
module.
- Around line 96-102: Remove the AWS Bedrock defaults from the provider-neutral
_failure_result function and its repeated caller values around the analysis
failure path. Preserve explicit provider and model identifiers when available,
but pass or return None for unknown clients so missing providers remain
unattributed; verify result_factory handling continues to accept a None
llm_provider.
In `@app/core/config.py`:
- Line 93: Correct the comment typo by changing “ReabbitMQ” to “RabbitMQ” in the
execution settings comment.
- Around line 125-141: Remove the separate blank-value checks for AWS_REGION and
BEDROCK_MODEL_ID, and rely on the existing required_values validation to process
them with the other production settings. Preserve the aggregated error reporting
so all missing or blank settings are reported together.
In `@app/infrastructure/llm/__init__.py`:
- Line 1: Correct the module docstring in app/infrastructure/llm/__init__.py by
changing the misspelled “ingreastructure” to “infrastructure.”
In `@tests/data_science/SMSModel/modeling/test_hybrid_thresholds.py`:
- Line 100: Update the comment near the hybrid-threshold test to replace the
provider-specific “Gemini 호출” wording with provider-neutral “LLM” terminology,
while preserving its meaning that stacking alone should avoid an LLM call.
In `@tests/infrastructure/rabbitmq/test_result_factory.py`:
- Around line 252-271: Update test_factory_reads_legacy_gemini_called_alias to
set result.text_analysis["llm_available"] to False after removing llm_called and
setting gemini_called. Keep the assertion focused on verifying that the factory
reads the legacy gemini_called alias rather than deriving llmCalled from
availability.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 489a5427-e143-421e-b4d9-37aa841dce6a
📒 Files selected for processing (50)
.env.example.env.prod.exampleREADME.mdapp/analysis/execution.pyapp/analysis/hybrid_policy.pyapp/analysis/rules/analyzer.pyapp/analysis/scoring.pyapp/analysis/service.pyapp/analysis/text/gemini_analyzer.pyapp/analysis/text/hybrid_analyzer.pyapp/analysis/text/llm_analyzer.pyapp/chat/service.pyapp/core/config.pyapp/infrastructure/gemini/__init__.pyapp/infrastructure/gemini/client.pyapp/infrastructure/llm/__init__.pyapp/infrastructure/llm/bedrock_client.pyapp/infrastructure/llm/factory.pyapp/infrastructure/llm/types.pyapp/infrastructure/mock_provider.pyapp/infrastructure/rabbitmq/result_factory.pyapp/infrastructure/rabbitmq/schemas.pydata_science/SMSModel/artifacts/stacking/gemini_validation_predictions.jsondata_science/SMSModel/modeling/hybrid_thresholds.pydata_science/SMSModel/run_hybrid_threshold_selection.pyrequirements.txtscripts/adversarial_test/daily_batch.pyscripts/adversarial_test/evaluate.pyscripts/adversarial_test/mutations.pyscripts/adversarial_test/rate_limit.pyscripts/benchmark/generate_llm_proxy_prompt.pyscripts/benchmark/measure_llm_timing.pyscripts/benchmark/run_llm_track.pytests/analysis/test_execution.pytests/analysis/test_hybrid_policy.pytests/analysis/test_router.pytests/analysis/test_service.pytests/analysis/text/test_gemini_analyzer.pytests/analysis/text/test_hybrid_analyzer.pytests/analysis/text/test_llm_analyzer.pytests/chat/test_router.pytests/chat/test_service.pytests/core/test_config.pytests/data_science/SMSModel/modeling/test_hybrid_thresholds.pytests/data_science/SMSModel/test_hybrid_threshold_selection.pytests/infrastructure/llm/__init__.pytests/infrastructure/llm/test_bedrock_client.pytests/infrastructure/llm/test_factory.pytests/infrastructure/rabbitmq/test_consumer.pytests/infrastructure/rabbitmq/test_result_factory.py
💤 Files with no reviewable changes (6)
- app/infrastructure/gemini/init.py
- tests/core/test_config.py
- data_science/SMSModel/artifacts/stacking/gemini_validation_predictions.json
- app/infrastructure/gemini/client.py
- tests/analysis/text/test_gemini_analyzer.py
- app/analysis/text/gemini_analyzer.py
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/benchmark/run_llm_track.py (1)
39-45: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winDo not require
error_messagefor mock results.If
result["is_mock"]is true, line 45 can raiseKeyErrorbecause a mock response does not need anerror_message. Read the error message safely and log a separate mock reason.Proposed fix
- if result.get("is_mock") or result.get("result", {}).get( - "error_message" - ): + error_message = result.get("result", {}).get("error_message") + if result.get("is_mock") or error_message: logger.warning( - "[LlmTrack] API 오류 응답 제외 id=%s error=%s", + "[LlmTrack] 응답 제외 id=%s error=%s", sample["id"], - result["result"]["error_message"], + error_message or "mock response", )🤖 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 `@scripts/benchmark/run_llm_track.py` around lines 39 - 45, Update the warning branch handling result exclusions to avoid directly indexing result["result"]["error_message"] when result.get("is_mock") is true. Safely retrieve the error message, and log a distinct mock reason for mock responses while preserving the existing API error details for non-mock error responses.
🤖 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 `@app/infrastructure/llm/bedrock_client.py`:
- Around line 102-122: Update the Bedrock call in the generate flow around
self._client.converse so it runs on a dedicated, bounded executor with bounded
in-flight admission rather than asyncio.to_thread’s shared default executor.
Ensure admission capacity is released only after the worker future actually
completes, including when asyncio.wait_for returns a timeout, while preserving
the existing LLM_TIMEOUT behavior.
---
Outside diff comments:
In `@scripts/benchmark/run_llm_track.py`:
- Around line 39-45: Update the warning branch handling result exclusions to
avoid directly indexing result["result"]["error_message"] when
result.get("is_mock") is true. Safely retrieve the error message, and log a
distinct mock reason for mock responses while preserving the existing API error
details for non-mock error responses.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c6b287c6-efde-4721-b5b8-1c69a205179c
📒 Files selected for processing (18)
.env.example.env.prod.exampleapp/analysis/hybrid_policy.pyapp/analysis/text/hybrid_analyzer.pyapp/analysis/text/llm_analyzer.pyapp/chat/service.pyapp/core/config.pyapp/infrastructure/llm/__init__.pyapp/infrastructure/llm/bedrock_client.pyapp/infrastructure/llm/types.pyscripts/adversarial_test/mutations.pyscripts/benchmark/measure_llm_timing.pyscripts/benchmark/run_llm_track.pytests/analysis/test_hybrid_policy.pytests/data_science/SMSModel/modeling/test_hybrid_thresholds.pytests/data_science/SMSModel/test_hybrid_threshold_selection.pytests/infrastructure/llm/test_bedrock_client.pytests/infrastructure/rabbitmq/test_result_factory.py
🚧 Files skipped from review as they are similar to previous changes (9)
- app/infrastructure/llm/init.py
- app/infrastructure/llm/types.py
- tests/infrastructure/rabbitmq/test_result_factory.py
- scripts/adversarial_test/mutations.py
- app/chat/service.py
- tests/data_science/SMSModel/modeling/test_hybrid_thresholds.py
- app/analysis/text/llm_analyzer.py
- scripts/benchmark/measure_llm_timing.py
- app/analysis/text/hybrid_analyzer.py
| response = await asyncio.wait_for( | ||
| asyncio.to_thread( | ||
| self._client.converse, | ||
| modelId=self.model_id, | ||
| system=[{"text": system_prompt}], | ||
| messages=bedrock_messages, | ||
| inferenceConfig={ | ||
| "maxTokens": ( | ||
| max_tokens | ||
| if max_tokens is not None | ||
| else settings.LLM_MAX_OUTPUT_TOKENS | ||
| ), | ||
| "temperature": ( | ||
| temperature | ||
| if temperature is not None | ||
| else settings.LLM_TEMPERATURE | ||
| ), | ||
| }, | ||
| ), | ||
| timeout=self._total_timeout_seconds, | ||
| ) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🌐 Web query:
According to the official Python asyncio documentation, does cancelling asyncio.wait_for around asyncio.to_thread stop the underlying thread function?
💡 Result:
According to the official Python asyncio documentation and the underlying implementation, cancelling an asyncio.wait_for task that wraps asyncio.to_thread does not stop the underlying thread [1][2][3]. When you cancel an asyncio task (such as the one returned by asyncio.wait_for), you are cancelling the coroutine that is managing the thread execution, not the thread itself [2][4]. Because Python provides no general mechanism to forcibly terminate an arbitrary thread from the outside, the thread will continue to run until the blocking function completes naturally [2][3][4]. Key points to understand: - Python threads are not interruptible by external mechanisms like asyncio cancellation [1][3][5]. - asyncio.to_thread is a wrapper around the event loop's run_in_executor method [2][6]. When the task is cancelled, the asyncio wrapper stops waiting for the thread to finish and raises a CancelledError in the event loop, but it cannot send a signal to the running thread to cease execution [2][3][4]. - If you need to stop a long-running function in a thread, you must design it to check for a cancellation signal, such as using a threading.Event object that the thread periodically checks during its execution [2]. Alternatively, using multiprocessing and a ProcessPoolExecutor may provide mechanisms to terminate child processes if true, external termination is required [5].
Citations:
- 1: https://bugs.python.org/issue43019
- 2: https://stackoverflow.com/questions/71416383/python-asyncio-cancelling-a-to-thread-task-wont-stop-the-thread
- 3: wait_for(to_thread)) does not work as expected. Extra documentation or fix needed. python/cpython#87185
- 4: https://stackoverflow.com/questions/71192465/how-to-timeout-asyncio-to-thread
- 5: https://stackoverflow.com/questions/76253359/is-it-possible-to-use-a-timeout-on-asyncio-to-thread-when-running-a-blocking-lon
- 6: https://github.com/python/cpython/blob/main/Lib/asyncio/threads.py
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- target file outline ---'
ast-grep outline app/infrastructure/llm/bedrock_client.py
printf '%s\n' '--- target implementation ---'
cat -n app/infrastructure/llm/bedrock_client.py | sed -n '1,180p'
printf '%s\n' '--- executor and timeout references ---'
rg -n -C 3 'to_thread|run_in_executor|wait_for|LLM_TIMEOUT|total_timeout|retry|Config|ThreadPoolExecutor' app tests 2>/dev/null || true
printf '%s\n' '--- project runtime and dependency declarations ---'
rg -n -C 2 'python_requires|requires-python|boto3|botocore|asyncio' pyproject.toml setup.cfg setup.py requirements*.txt 2>/dev/null || trueRepository: SafeFam/SafeFam_AI
Length of output: 50374
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- settings relevant to Bedrock limits ---'
cat -n app/core/config.py | sed -n '35,80p'
printf '%s\n' '--- Bedrock client construction and call sites ---'
rg -n -C 4 'BedrockLlmClient|generate\(' app tests/infrastructure/llm tests/analysis/text
printf '%s\n' '--- existing Bedrock tests around timeout and concurrency ---'
cat -n tests/infrastructure/llm/test_bedrock_client.py | sed -n '1,190p'
printf '%s\n' '--- runtime cancellation probe ---'
python3 - <<'PY'
import asyncio
import threading
import time
started = threading.Event()
finished = threading.Event()
def blocking_call():
started.set()
time.sleep(0.20)
finished.set()
return "done"
async def main():
task = asyncio.create_task(
asyncio.wait_for(asyncio.to_thread(blocking_call), timeout=0.02)
)
await asyncio.to_thread(started.wait, 1)
try:
await task
except TimeoutError:
print("wait_for_result=TimeoutError")
print(f"finished_immediately={finished.is_set()}")
await asyncio.to_thread(finished.wait, 1)
print(f"finished_later={finished.is_set()}")
asyncio.run(main())
PYRepository: SafeFam/SafeFam_AI
Length of output: 24210
🏁 Script executed:
#!/bin/bash
set -e
python3 - <<'PY'
import asyncio
import threading
import time
started = threading.Event()
finished = threading.Event()
def blocking_call():
started.set()
time.sleep(0.20)
finished.set()
return "done"
async def main():
task = asyncio.create_task(
asyncio.wait_for(asyncio.to_thread(blocking_call), timeout=0.02)
)
await asyncio.to_thread(started.wait, 1)
try:
await task
except asyncio.TimeoutError:
print("wait_for_result=TimeoutError")
print(f"finished_immediately={finished.is_set()}")
await asyncio.to_thread(finished.wait, 1)
print(f"finished_later={finished.is_set()}")
asyncio.run(main())
PYRepository: SafeFam/SafeFam_AI
Length of output: 232
🌐 Web query:
site:docs.python.org/3/library/asyncio-task.html asyncio.to_thread cancellation wait_for
💡 Result:
When using asyncio.to_thread with asyncio.wait_for, canceling the task associated with asyncio.to_thread will cancel the future returned by the call, but it cannot stop the underlying thread from executing [1]. In Python's asyncio, to_thread is designed to run blocking IO-bound functions in a separate thread to avoid blocking the event loop [1]. While the asyncio task waiting for the result can be cancelled—for example, via a timeout in asyncio.wait_for—this cancellation only affects the event loop's side of the operation [1]. The thread spawned by to_thread will continue to run until the blocking function completes, as Python does not provide a safe way to forcefully terminate a running thread from another thread [1]. In summary: 1. If asyncio.wait_for reaches its timeout, it will cancel the asyncio task waiting for to_thread, raising a TimeoutError [1]. 2. The event loop will stop waiting for the thread's result, but the thread itself will continue to execute the function until it finishes naturally [1]. 3. Resources held by that thread will not be released until the function completes, and any result or exception generated by the thread after the timeout will be ignored by the asyncio task that was already cancelled [1].
Citations:
Prevent timed-out Bedrock calls from occupying default executor workers.
asyncio.wait_for cancels the await, but it does not stop a running asyncio.to_thread worker. The synchronous converse call can continue through its retry budget after generate returns LLM_TIMEOUT.
During a Bedrock outage, repeated timeouts can occupy the shared default executor and delay unrelated asyncio.to_thread work. Use a dedicated bounded executor and bounded in-flight admission. Release capacity only when the worker future completes.
🤖 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 `@app/infrastructure/llm/bedrock_client.py` around lines 102 - 122, Update the
Bedrock call in the generate flow around self._client.converse so it runs on a
dedicated, bounded executor with bounded in-flight admission rather than
asyncio.to_thread’s shared default executor. Ensure admission capacity is
released only after the worker future actually completes, including when
asyncio.wait_for returns a timeout, while preserving the existing LLM_TIMEOUT
behavior.
1 participant
📝 개요
기존 Gemini 기반 스미싱 분석기와 채팅 서비스를 AWS Bedrock 기반 Anthropic Claude Haiku로 교체했습니다.
Stacking 모델이 확신하는 구간에서는 LLM 호출을 생략하고, 불확실한 구간에서만 Bedrock을 호출하는 기존 하이브리드 정책을 공급자 중립적인 구조로 변경했습니다.
LLM 호출 실패 시에는 Stacking 결과로 fallback하고, 모든 텍스트 분석 엔진이 실패한 경우
UNKNOWN을 반환하는 fail-safe 정책을 유지했습니다.🔗 관련 이슈
🎯 주요 변경 사항
AWS Bedrock Runtime
ConverseAPI 클라이언트 구현AWS_REGION,AWS_PROFILE,BEDROCK_MODEL_ID설정을 지원합니다.Gemini 스미싱 분석기를 공급자 중립적인 LLM 분석기로 교체
gemini_analyzer.py를 제거하고llm_analyzer.py를 추가했습니다.risk_score,grade,tone_analysis,evidence,reason,error_message형식으로 검증·정규화합니다.UNKNOWN을 반환합니다.Hybrid 라우팅 구조를 LLM 기준으로 일반화
ConditionalGeminiPolicy→ConditionalLlmPolicyGEMINI_REVIEW→LLM_REVIEWGEMINI_FALLBACK→LLM_FALLBACKgemini_analyzer→llm_analyzergemini_called→llm_calledgemini_available→llm_availableTEXT:GEMINI→TEXT:LLMdecision_source=GEMINI→decision_source=LLM기존 SafeFam_BE와의 한시적인 하위 호환성 유지
gemini,gemini_called,gemini_availablegeminiCalledGEMINI,STACKING_GEMINIEnumllm*필드를 표준으로 사용합니다.RabbitMQ 분석 결과에 공급자 중립 필드를 추가
llmCalledllmProviderllmModelLLM,STACKING_LLM분석 방식 EnumTEXT:LLM으로 발행합니다.채팅 서비스를 Gemini에서 Bedrock으로 교체
get_llm_client()를 사용합니다.user,assistantrole을 그대로 전달합니다.Validation 캐시를 공급자 중립 구조로 일반화
gemini_validation_predictions.json→llm_validation_predictions.json--collect-gemini→--collect-llmBenchmark 및 adversarial test 도구를 Bedrock 기반 LLM 호출로 변경
📸 사진
✅ PR 체크리스트
uvicorn구동 또는 테스트 코드)를 통과했습니다.Summary by CodeRabbit
New Features
Improvements
Documentation