Feat/add caching#24
Open
akashd56 wants to merge 4 commits into
Open
Conversation
|
@akashd56 is attempting to deploy a commit to the ClaraClency Team on Vercel. A member of the Team first needs to authorize it. |
✅ Deploy Preview for livedit canceled.
|
📝 WalkthroughWalkthroughThe backend updates runtime configuration, database persistence, authentication, caching, media editing, payments, AI ingestion, director workflows, structured logging, and related tests. ChangesBackend platform and video workflows
Sequence Diagram(s)sequenceDiagram
participant Client
participant AnalyzeVideoAPI
participant analyze_video_task
participant Redis
participant Database
Client->>AnalyzeVideoAPI: Submit video and skip_cache option
AnalyzeVideoAPI->>analyze_video_task: Queue analysis job
analyze_video_task->>Redis: Check MD5 cache key
Redis-->>analyze_video_task: Return cached result or miss
analyze_video_task->>Database: Store job result and cache status
AnalyzeVideoAPI-->>Client: Return job metadata
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing Touches🧪 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: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
LiveEditBackend/app.py (1)
2017-2041: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftAdd a lifecycle for render files.
The uploaded video/audio temporary files are never removed, while rendered outputs and
_rendered_video_storeentries have no expiry. Repeated renders will grow disk and process memory without bound. Delete inputs infinallyand expire rendered artifacts after a configured retention period.🤖 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 `@LiveEditBackend/app.py` around lines 2017 - 2041, The render flow around render_video_from_plan must manage artifact lifecycles: wrap temporary input/audio creation and rendering in a finally block that removes input_path and audio_path when present, and add configured retention-based expiry for rendered output files and corresponding _rendered_video_store entries. Ensure expired artifacts are deleted from disk and memory while preserving active render results.LiveEditBackend/video_tasks.py (1)
302-355: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winMake Redis strictly best-effort for analysis jobs.
LiveEditBackend/video_tasks.py#L302-L355: isolate cache reads, decoding, counters, and writes. Connection errors or malformed entries should fall through to Gemini, and a failed cache write must not discard a completed analysis.LiveEditBackend/tests/test_analysis_cache.py#L20-L53: add failure modes forget,incr, andsetex, plus malformed cached JSON, and verify successful fallback analysis.🤖 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 `@LiveEditBackend/video_tasks.py` around lines 302 - 355, The analysis cache flow in the analysis job must be strictly best-effort: in LiveEditBackend/video_tasks.py lines 302-355, isolate Redis get, cached JSON decoding, hit/miss counters, and setex so Redis errors or malformed entries fall through to analysis, while a failed write still returns the completed analysis; in LiveEditBackend/tests/test_analysis_cache.py lines 20-53, add coverage for get, incr, setex, and malformed-JSON failures and verify fallback analysis succeeds.
🤖 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 `@LiveEditBackend/app.py`:
- Around line 83-95: Update the after_request CORS handler to authorize
credentialed responses only when origin is in ALLOWED_ORIGINS. Remove the
origin.endswith(".vercel.app") condition while preserving the existing headers
and response flow.
- Around line 540-546: Implement server-verifiable authentication across the
listed sites: in LiveEditBackend/app.py lines 540-546 and 584-590, issue
persistently tracked sessions or signed credentials at signup and login; in
lines 1014-1030, require authenticated administrator authorization before
clearing caches; in lines 1473-1545, derive subscription and transaction
identity from the authenticated user rather than a caller-supplied email; update
LiveEditBackend/tests/test_analysis_cache.py lines 318-371 to require
authorization and cover unauthenticated and unauthorized cache-clear responses.
- Around line 1584-1586: Update the requests.post call in the Imagen request
flow to pass timeout=(5, 60), bounding connection and response-read time while
preserving the existing URL, headers, and payload.
- Around line 636-669: Harden the outbound fetch in import_audio_from_url by
allowing only approved URL destinations, resolving and rejecting private or
reserved IP addresses, and validating every redirect against the same checks.
Enforce a strict maximum response size while streaming the download and require
an allowed audio content type before writing to the sounds directory; preserve
the existing required-field validation and import flow.
In `@LiveEditBackend/utils/logger.py`:
- Around line 180-184: Remove request and response body logging from the logger
flow, including the request payload block around request.is_json and the
corresponding response-body handling near the referenced code. Do not rely on
size limits or key-based sanitization; preserve only non-body request/response
metadata in the logs.
- Around line 153-155: Update the plaintext logging formatter configuration
around handler.setFormatter to sanitize rendered log messages by neutralizing
embedded carriage-return and line-feed control characters before formatting.
Apply the same sanitization to the other plaintext logging paths identified near
the additional locations, while leaving JSON_LOGS=true behavior unchanged.
Ensure request paths and exception text cannot create additional log records.
- Around line 123-124: Update the traceback handling in the logger’s record
serialization to sanitize the output of formatException(record.exc_info) using
the same path-sanitization logic applied to normal messages before assigning
log_data["traceback"]. Preserve the existing traceback capture behavior while
ensuring absolute source paths are removed.
- Around line 134-141: Update the Sentry initialization block guarded by
SENTRY_DSN so traces_sample_rate and profiles_sample_rate are read from
environment configuration rather than hardcoded to 1.0. Define conservative
default values and parse the environment-provided rates before passing them to
sentry_sdk.init.
In `@LiveEditBackend/video_tasks.py`:
- Around line 292-293: Update the cache-key construction around file_hash in the
video analysis flow to include the analysis request identity, including
user_prompt and the relevant model/schema identifiers. Apply the same change to
the additional cache-key path near the later referenced block, ensuring
identical video data with different requests cannot share cached results.
- Around line 338-346: The video analysis path currently returns a hardcoded
result on cache misses. In LiveEditBackend/video_tasks.py lines 338-346, restore
the call to call_gemini_with_retry and pass its response through
parse_model_response before caching and returning it; update
LiveEditBackend/tests/test_analysis_cache.py lines 94-251 to mock the Gemini
call, verify the mocked analysis is cached, and verify cache hits do not invoke
the mock.
---
Outside diff comments:
In `@LiveEditBackend/app.py`:
- Around line 2017-2041: The render flow around render_video_from_plan must
manage artifact lifecycles: wrap temporary input/audio creation and rendering in
a finally block that removes input_path and audio_path when present, and add
configured retention-based expiry for rendered output files and corresponding
_rendered_video_store entries. Ensure expired artifacts are deleted from disk
and memory while preserving active render results.
In `@LiveEditBackend/video_tasks.py`:
- Around line 302-355: The analysis cache flow in the analysis job must be
strictly best-effort: in LiveEditBackend/video_tasks.py lines 302-355, isolate
Redis get, cached JSON decoding, hit/miss counters, and setex so Redis errors or
malformed entries fall through to analysis, while a failed write still returns
the completed analysis; in LiveEditBackend/tests/test_analysis_cache.py lines
20-53, add coverage for get, incr, setex, and malformed-JSON failures and verify
fallback analysis succeeds.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0d4534c5-043a-4fea-b48a-e97a5e57d779
📒 Files selected for processing (9)
.python-versionLiveEditBackend/.python-versionLiveEditBackend/app.pyLiveEditBackend/celery_config.pyLiveEditBackend/requirements.txtLiveEditBackend/tests/test_analysis_cache.pyLiveEditBackend/utils/logger.pyLiveEditBackend/video_tasks.pypyrightconfig.json
Comment on lines
83
to
95
| # Additional CORS headers for all responses | ||
| @app.after_request | ||
| def after_request(response): | ||
| origin = request.headers.get('Origin') | ||
| if origin in ALLOWED_ORIGINS or (origin and origin.endswith('.vercel.app')): | ||
| response.headers['Access-Control-Allow-Origin'] = origin | ||
| response.headers['Access-Control-Allow-Credentials'] = 'true' | ||
| response.headers['Access-Control-Allow-Methods'] = 'GET, POST, PUT, DELETE, OPTIONS' | ||
| response.headers['Access-Control-Allow-Headers'] = 'Content-Type, Authorization' | ||
| response.headers['Access-Control-Max-Age'] = '3600' | ||
| origin = request.headers.get("Origin") | ||
| if origin in ALLOWED_ORIGINS or (origin and origin.endswith(".vercel.app")): | ||
| response.headers["Access-Control-Allow-Origin"] = origin | ||
| response.headers["Access-Control-Allow-Credentials"] = "true" | ||
| response.headers["Access-Control-Allow-Methods"] = ( | ||
| "GET, POST, PUT, DELETE, OPTIONS" | ||
| ) | ||
| response.headers["Access-Control-Allow-Headers"] = "Content-Type, Authorization" | ||
| response.headers["Access-Control-Max-Age"] = "3600" | ||
| return response |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Restrict credentialed CORS to exact trusted origins.
Line 87 reflects any *.vercel.app origin while enabling credentials. An attacker-controlled Vercel deployment is therefore treated as trusted; use the explicit allowlist only.
🤖 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 `@LiveEditBackend/app.py` around lines 83 - 95, Update the after_request CORS
handler to authorize credentialed responses only when origin is in
ALLOWED_ORIGINS. Remove the origin.endswith(".vercel.app") condition while
preserving the existing headers and response flow.
Comment on lines
540
to
+546
| # Generate token (in production, use JWT) | ||
| token = secrets.token_urlsafe(32) | ||
|
|
||
| cur.close() | ||
| conn.close() | ||
|
|
||
| return jsonify({'success': True, 'token': token, 'email': email}), 201 | ||
| return jsonify({"success": True, "token": token, "email": email}), 201 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | 🏗️ Heavy lift
Implement server-verifiable authentication before exposing sensitive routes.
The returned tokens are neither persisted nor signed, and no route validates them.
LiveEditBackend/app.py#L540-L546: issue a persistently tracked session or signed token at signup.LiveEditBackend/app.py#L584-L590: issue the same verifiable credential at login.LiveEditBackend/app.py#L1014-L1030: restrict cache clearing to administrators.LiveEditBackend/app.py#L1473-L1545: derive user identity from authentication instead of accepting an arbitrary email for subscription and transaction data.LiveEditBackend/tests/test_analysis_cache.py#L318-L371: require authorization and cover unauthenticated/unauthorized cache-clear responses.
📍 Affects 2 files
LiveEditBackend/app.py#L540-L546(this comment)LiveEditBackend/app.py#L584-L590LiveEditBackend/app.py#L1014-L1030LiveEditBackend/app.py#L1473-L1545LiveEditBackend/tests/test_analysis_cache.py#L318-L371
🤖 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 `@LiveEditBackend/app.py` around lines 540 - 546, Implement server-verifiable
authentication across the listed sites: in LiveEditBackend/app.py lines 540-546
and 584-590, issue persistently tracked sessions or signed credentials at signup
and login; in lines 1014-1030, require authenticated administrator authorization
before clearing caches; in lines 1473-1545, derive subscription and transaction
identity from the authenticated user rather than a caller-supplied email; update
LiveEditBackend/tests/test_analysis_cache.py lines 318-371 to require
authorization and cover unauthenticated and unauthorized cache-clear responses.
Comment on lines
+636
to
669
| @app.route("/api/audio-effects/import", methods=["POST"]) | ||
| def import_audio_from_url(): | ||
| """Download audio from URL and add to library""" | ||
| import requests | ||
|
|
||
| try: | ||
| data = request.get_json() | ||
| url = data.get('url') | ||
| name = data.get('name') | ||
| category = data.get('category', 'effect') | ||
| description = data.get('description', '') | ||
| tags = data.get('tags', []) | ||
| url = data.get("url") | ||
| name = data.get("name") | ||
| category = data.get("category", "effect") | ||
| description = data.get("description", "") | ||
| tags = data.get("tags", []) | ||
|
|
||
| if not url or not name: | ||
| return jsonify({'error': 'URL and name are required'}), 400 | ||
| return jsonify({"error": "URL and name are required"}), 400 | ||
|
|
||
| # Download the file | ||
| response = requests.get(url, timeout=30) | ||
| response.raise_for_status() | ||
|
|
||
| # Generate filename from name | ||
| import re | ||
| filename = re.sub(r'[^\w\s-]', '', name.lower()) | ||
| filename = re.sub(r'[-\s]+', '_', filename) | ||
|
|
||
| filename = re.sub(r"[^\w\s-]", "", name.lower()) | ||
| filename = re.sub(r"[-\s]+", "_", filename) | ||
| filename = f"{filename}.mp3" | ||
|
|
||
| # Save to sounds directory | ||
| sounds_dir = os.path.join(os.path.dirname(__file__), 'sounds') | ||
| sounds_dir = os.path.join(os.path.dirname(__file__), "sounds") | ||
| os.makedirs(sounds_dir, exist_ok=True) | ||
| file_path = os.path.join(sounds_dir, filename) | ||
| with open(file_path, 'wb') as f: | ||
|
|
||
| with open(file_path, "wb") as f: | ||
| f.write(response.content) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant region with line numbers.
sed -n '600,740p' LiveEditBackend/app.py | cat -n
printf '\n==== SEARCH: audio serving routes ====\n'
rg -n "send_from_directory|sounds_dir|/api/audio|audio-serving|serve.*sound|static.*sounds|import_audio_from_url|requests.get\\(" LiveEditBackend/app.pyRepository: clencyc/LiveEdit
Length of output: 6426
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the file structure for app.py so we can inspect surrounding helpers if needed.
ast-grep outline LiveEditBackend/app.py --view expandedRepository: clencyc/LiveEdit
Length of output: 2442
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check whether these routes are protected by auth middleware/decorators.
rg -n "@(login_required|auth_required|require_auth)|before_request|Authorization|request\.headers|get\('Authorization'|jwt|verify_password|current_user" LiveEditBackend/app.pyRepository: clencyc/LiveEdit
Length of output: 503
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect nearby code for any auth-related wrapper or validation helper that might apply to the audio routes.
sed -n '1,130p' LiveEditBackend/app.py | cat -nRepository: clencyc/LiveEdit
Length of output: 4660
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the route definitions around the import endpoint in context.
sed -n '620,705p' LiveEditBackend/app.py | cat -nRepository: clencyc/LiveEdit
Length of output: 3609
Block caller-controlled outbound fetches in the audio import endpoint.
It fetches an arbitrary URL server-side and writes the response into a publicly served sounds directory, so it can reach internal/metadata resources and expose the result to anyone. Restrict destinations to an allowlist, reject private/reserved IPs after DNS resolution and redirects, and enforce response size/type limits.
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 667-667: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(file_path, "wb")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
[warning] 652-652: Request-controlled URL passed to requests; validate against an allowlist to prevent SSRF.
Context: requests.get(url, timeout=30)
Note: [CWE-918] Server-Side Request Forgery (SSRF).
(ssrf-requests)
🤖 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 `@LiveEditBackend/app.py` around lines 636 - 669, Harden the outbound fetch in
import_audio_from_url by allowing only approved URL destinations, resolving and
rejecting private or reserved IP addresses, and validating every redirect
against the same checks. Enforce a strict maximum response size while streaming
the download and require an allowed audio content type before writing to the
sounds directory; preserve the existing required-field validation and import
flow.
Source: Linters/SAST tools
Comment on lines
+1584
to
+1586
| response = requests.post( | ||
| f"{url}?key={API_KEY}", headers=headers, json=payload | ||
| ) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the file structure around the target area first
ast-grep outline LiveEditBackend/app.py --view expanded || true
# Read the relevant section with line numbers
sed -n '1540,1615p' LiveEditBackend/app.py | cat -n
# Search for other requests.post / requests.request calls and timeout usage in the backend
rg -n "requests\.(post|request|get|put|delete|patch)\(" LiveEditBackend -g '*.py' || true
rg -n "timeout\s*=" LiveEditBackend -g '*.py' || trueRepository: clencyc/LiveEdit
Length of output: 6088
Add a timeout to the Imagen request.
A stalled upstream connection can tie up a Flask worker indefinitely; timeout=(5, 60) keeps connect/read bounded.
🧰 Tools
🪛 Ruff (0.15.21)
[error] 1584-1584: Probable use of requests call without timeout
(S113)
🤖 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 `@LiveEditBackend/app.py` around lines 1584 - 1586, Update the requests.post
call in the Imagen request flow to pass timeout=(5, 60), bounding connection and
response-read time while preserving the existing URL, headers, and payload.
Source: Linters/SAST tools
Comment on lines
+123
to
+124
| if record.exc_info: | ||
| log_data["traceback"] = self.formatException(record.exc_info) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Sanitize tracebacks before serializing them.
formatException() bypasses the path sanitization applied to normal messages, exposing absolute source paths in logs.
Proposed fix
if record.exc_info:
- log_data["traceback"] = self.formatException(record.exc_info)
+ log_data["traceback"] = sanitize_string(
+ self.formatException(record.exc_info)
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if record.exc_info: | |
| log_data["traceback"] = self.formatException(record.exc_info) | |
| if record.exc_info: | |
| log_data["traceback"] = sanitize_string( | |
| self.formatException(record.exc_info) | |
| ) |
🤖 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 `@LiveEditBackend/utils/logger.py` around lines 123 - 124, Update the traceback
handling in the logger’s record serialization to sanitize the output of
formatException(record.exc_info) using the same path-sanitization logic applied
to normal messages before assigning log_data["traceback"]. Preserve the existing
traceback capture behavior while ensuring absolute source paths are removed.
Comment on lines
+134
to
+141
| # Sentry integration | ||
| if SENTRY_DSN: | ||
| sentry_sdk.init( | ||
| dsn=SENTRY_DSN, | ||
| integrations=[FlaskIntegration()], | ||
| traces_sample_rate=1.0, | ||
| profiles_sample_rate=1.0, | ||
| ) |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
set -e
printf '== logger.py context ==\n'
nl -ba LiveEditBackend/utils/logger.py | sed -n '1,220p'
printf '\n== sentry env search ==\n'
rg -n "SENTRY_(TRACES|PROFILES)_SAMPLE_RATE|SENTRY_DSN|sentry_sdk\.init|FlaskIntegration" -S .Repository: clencyc/LiveEdit
Length of output: 676
🏁 Script executed:
set -e
sed -n '125,150p' LiveEditBackend/utils/logger.py | cat -nRepository: clencyc/LiveEdit
Length of output: 1213
Avoid hardcoding 100% Sentry sampling traces_sample_rate=1.0 and profiles_sample_rate=1.0 send every transaction and profile whenever SENTRY_DSN is set. Make both rates environment-controlled with conservative defaults before this reaches production.
🤖 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 `@LiveEditBackend/utils/logger.py` around lines 134 - 141, Update the Sentry
initialization block guarded by SENTRY_DSN so traces_sample_rate and
profiles_sample_rate are read from environment configuration rather than
hardcoded to 1.0. Define conservative default values and parse the
environment-provided rates before passing them to sentry_sdk.init.
Comment on lines
+153
to
+155
| handler.setFormatter(logging.Formatter( | ||
| "[%(asctime)s] %(levelname)s in %(name)s: %(message)s" | ||
| )) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Neutralize control characters in plaintext logs.
When JSON_LOGS=false, request paths and exception text are emitted unchanged. Embedded CR/LF characters can split entries and forge log records; sanitize the rendered message before formatting.
Proposed fix
+class SanitizingFormatter(logging.Formatter):
+ def format(self, record):
+ original_msg, original_args = record.msg, record.args
+ try:
+ message = sanitize_string(record.getMessage())
+ record.msg = message.replace("\r", "\\r").replace("\n", "\\n")
+ record.args = ()
+ return super().format(record)
+ finally:
+ record.msg, record.args = original_msg, original_args
+
...
- handler.setFormatter(logging.Formatter(
+ handler.setFormatter(SanitizingFormatter(
"[%(asctime)s] %(levelname)s in %(name)s: %(message)s"
))Also applies to: 187-187, 238-241
🤖 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 `@LiveEditBackend/utils/logger.py` around lines 153 - 155, Update the plaintext
logging formatter configuration around handler.setFormatter to sanitize rendered
log messages by neutralizing embedded carriage-return and line-feed control
characters before formatting. Apply the same sanitization to the other plaintext
logging paths identified near the additional locations, while leaving
JSON_LOGS=true behavior unchanged. Ensure request paths and exception text
cannot create additional log records.
Source: Linters/SAST tools
Comment on lines
+180
to
+184
| # Log request (avoid payload if it's too big, e.g. files, but log small JSON payloads) | ||
| if request.is_json and request.content_length and request.content_length < 10000: | ||
| try: | ||
| log_payload["body"] = sanitize_data(request.get_json()) | ||
| except Exception: |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Do not log arbitrary request and response bodies.
The 10 KB threshold limits size, not sensitivity. Key-based masking still permits emails, payment metadata, prompts, and other PII to enter persistent logs. Remove body logging or use an explicit endpoint/field allowlist.
Safe default
- if request.is_json and request.content_length and request.content_length < 10000:
- try:
- log_payload["body"] = sanitize_data(request.get_json())
- except Exception:
- pass
...
- if response.is_json and response.content_length and response.content_length < 10000:
- try:
- log_payload["response_body"] = sanitize_data(response.get_json())
- except Exception:
- passAlso applies to: 204-208
🧰 Tools
🪛 Ruff (0.15.21)
[warning] 184-184: Do not catch blind exception: Exception
(BLE001)
🤖 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 `@LiveEditBackend/utils/logger.py` around lines 180 - 184, Remove request and
response body logging from the logger flow, including the request payload block
around request.is_json and the corresponding response-body handling near the
referenced code. Do not rely on size limits or key-based sanitization; preserve
only non-body request/response metadata in the logs.
Comment on lines
+292
to
+293
| file_hash = hashlib.md5(video_data).hexdigest() | ||
| cache_key = f"liveedit:analysis:{file_hash}" |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Include the analysis request in cache identity.
The cached result depends on user_prompt, but the key depends only on video bytes. Uploading the same video with a different prompt returns the first prompt's analysis, potentially across users. Include prompt/model/schema identity in the cached variant or validate that metadata before accepting a hit.
Also applies to: 334-336
🧰 Tools
🪛 Ruff (0.15.21)
[error] 292-292: Probable use of insecure hash functions in hashlib: md5
(S324)
🤖 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 `@LiveEditBackend/video_tasks.py` around lines 292 - 293, Update the cache-key
construction around file_hash in the video analysis flow to include the analysis
request identity, including user_prompt and the relevant model/schema
identifiers. Apply the same change to the additional cache-key path near the
later referenced block, ensuring identical video data with different requests
cannot share cached results.
Comment on lines
+338
to
+346
| # response = call_gemini_with_retry( | ||
| # contents=[video_part, analysis_prompt], model=TEXT_MODEL_NAME, max_retries=3 | ||
| # ) | ||
| # result = parse_model_response(response) | ||
| result = { | ||
| "summary": "test cached analysis", | ||
| "key_events": [], | ||
| "edit_plan": [], | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Restore real Gemini analysis; the tests currently validate a production stub.
LiveEditBackend/video_tasks.py#L338-L346: restorecall_gemini_with_retry(...)andparse_model_response(...); every cache miss currently returns the same fake result without analyzing the video.LiveEditBackend/tests/test_analysis_cache.py#L94-L251: mock the Gemini call and assert that its mocked result is cached; also assert that cache hits do not invoke it.
📍 Affects 2 files
LiveEditBackend/video_tasks.py#L338-L346(this comment)LiveEditBackend/tests/test_analysis_cache.py#L94-L251
🤖 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 `@LiveEditBackend/video_tasks.py` around lines 338 - 346, The video analysis
path currently returns a hardcoded result on cache misses. In
LiveEditBackend/video_tasks.py lines 338-346, restore the call to
call_gemini_with_retry and pass its response through parse_model_response before
caching and returning it; update LiveEditBackend/tests/test_analysis_cache.py
lines 94-251 to mock the Gemini call, verify the mocked analysis is cached, and
verify cache hits do not invoke the mock.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements Redis caching for video analysis results to reduce repeated Gemini API calls and lower API costs.
Closes #7.
Changes
CACHE_TTL)ENABLE_ANALYSIS_CACHE)?skip_cache=trueGET /api/cache/statsTesting
skip_cache=trueSummary by CodeRabbit
New Features
Bug Fixes
Refactor