fix(api): replace assert with explicit exceptions in db_manager and u… - #6234
Open
shafeeq27edu-ai wants to merge 2 commits into
Open
fix(api): replace assert with explicit exceptions in db_manager and u…#6234shafeeq27edu-ai wants to merge 2 commits into
shafeeq27edu-ai wants to merge 2 commits into
Conversation
dosubot
Bot
added
the
size:S
|
@shafeeq27edu-ai is attempting to deploy a commit to the agenta projects Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review. 📝 WalkthroughSummary by CodeRabbit
WalkthroughThe changes replace assertion-based workspace validation with an explicit ChangesExplicit error handling
Estimated code review effort: 1 (Trivial) | ~5 minutes Merge Risk: 🟡 Moderate · up to The PR makes validation reliable under optimized Python execution, but the missing-user path still appears able to return a 500 instead of the intended 404 because the database lookup raises first. This bounded correctness issue should be fixed or explicitly accepted before merging. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: f83a0b0d-b3e4-4dbe-8b05-ca19bd9ec79f
📒 Files selected for processing (2)
api/oss/src/routers/user_profile.pyapi/oss/src/services/db_manager.py
Included review availability: Your plan provides up to 8 included reviews per hour; 5 remain after this review.
Comment on lines
+47
to
+51
| if user is None: | ||
| raise HTTPException( | ||
| status_code=404, | ||
| detail="User not found. Please ensure that the user_id is specified correctly.", | ||
| ) |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- candidate files ---'
fd -i 'user_profile.py|db_manager.py' api
printf '%s\n' '--- endpoint context ---'
sed -n '1,110p' api/oss/src/routers/user_profile.py
printf '%s\n' '--- database helper context ---'
sed -n '1200,1280p' api/oss/src/services/db_manager.py
printf '%s\n' '--- NoResultFound handlers and usages ---'
rg -n -S 'NoResultFound|no_result_found|exception_handler|HTTPException' api/oss/src api/oss/tests api 2>/dev/null | head -n 300Repository: Agenta-AI/agenta
Length of output: 36312
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- APIRouter and exception registration ---'
rg -n -S 'class APIRouter|APIRouter\s*=|add_exception_handler|NoResultFound|SQLAlchemyError|ExceptionMiddleware' api/oss/src api/oss/tests | head -n 250
printf '%s\n' '--- user profile tests ---'
sed -n '1,260p' api/oss/tests/legacy/old_tests/variants_user_profile_router/test_user_profile.py
printf '%s\n' '--- user profile remaining router code ---'
sed -n '100,165p' api/oss/src/routers/user_profile.py
printf '%s\n' '--- imports for NoResultFound ---'
rg -n -S 'from sqlalchemy.exc import .*NoResultFound|import NoResultFound|NoResultFound' api/oss/src/services/db_manager.py api/oss/src | head -n 120
printf '%s\n' '--- source verifier ---'
python3 - <<'PY'
from pathlib import Path
import ast
router_path = Path("api/oss/src/routers/user_profile.py")
db_path = Path("api/oss/src/services/db_manager.py")
router = ast.parse(router_path.read_text())
db = ast.parse(db_path.read_text())
def find_function(tree, name):
for node in ast.walk(tree):
if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)) and node.name == name:
return node
raise LookupError(name)
endpoint = find_function(router, "user_profile")
helper = find_function(db, "get_user_with_id")
calls_helper = any(
isinstance(node, ast.Call)
and isinstance(node.func, ast.Attribute)
and node.func.attr == "get_user_with_id"
for node in ast.walk(endpoint)
)
raises_no_result = any(
isinstance(node, ast.Raise)
and isinstance(node.exc, ast.Call)
and isinstance(node.exc.func, ast.Name)
and node.exc.func.id == "NoResultFound"
for node in ast.walk(helper)
)
none_guard = any(
isinstance(node, ast.If)
and isinstance(node.test, ast.Compare)
and len(node.test.ops) == 1
and isinstance(node.test.ops[0], ast.Is)
and isinstance(node.test.comparators[0], ast.Constant)
and node.test.comparators[0].value is None
for node in ast.walk(endpoint)
)
no_result_handler = "NoResultFound" in router_path.read_text()
print({
"user_profile_calls_get_user_with_id": calls_helper,
"get_user_with_id_raises_NoResultFound": raises_no_result,
"user_profile_has_none_guard": none_guard,
"user_profile_mentions_NoResultFound": no_result_handler,
})
PYRepository: Agenta-AI/agenta
Length of output: 15668
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- focused exception-registration search ---'
rg -n -S -g '*.py' 'add_exception_handler|exception_handlers|NoResultFound|@.*exception' api/oss/src api/oss/tests || true
printf '%s\n' '--- custom router definition ---'
rg -n -S -g '*.py' 'class APIRouter|def APIRouter' api/oss/src
printf '%s\n' '--- focused test file ---'
wc -l api/oss/tests/legacy/old_tests/variants_user_profile_router/test_user_profile.py
cat -n api/oss/tests/legacy/old_tests/variants_user_profile_router/test_user_profile.py
printf '%s\n' '--- test-runner references ---'
rg -n -S 'py-run-tests|run-tests.py' README.md CONTRIBUTING.md api/Makefile api/pyproject.toml api 2>/dev/null | head -n 160 || true
printf '%s\n' '--- verifier result ---'
python3 - <<'PY'
from pathlib import Path
import ast
router_path = Path("api/oss/src/routers/user_profile.py")
db_path = Path("api/oss/src/services/db_manager.py")
router_text = router_path.read_text()
db_text = db_path.read_text()
router = ast.parse(router_text)
db = ast.parse(db_text)
def find_function(tree, name):
for node in ast.walk(tree):
if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)) and node.name == name:
return node
raise LookupError(name)
endpoint = find_function(router, "user_profile")
helper = find_function(db, "get_user_with_id")
calls_helper = any(
isinstance(node, ast.Call)
and isinstance(node.func, ast.Attribute)
and node.func.attr == "get_user_with_id"
for node in ast.walk(endpoint)
)
raises_no_result = any(
isinstance(node, ast.Raise)
and isinstance(node.exc, ast.Call)
and isinstance(node.exc.func, ast.Name)
and node.exc.func.id == "NoResultFound"
for node in ast.walk(helper)
)
has_try_except_no_result = any(
isinstance(node, ast.Try)
and any(
isinstance(handler.type, ast.Name) and handler.type.id == "NoResultFound"
for handler in node.handlers
)
for node in ast.walk(endpoint)
)
has_none_guard = any(
isinstance(node, ast.If)
and isinstance(node.test, ast.Compare)
and len(node.test.ops) == 1
and isinstance(node.test.ops[0], ast.Is)
and len(node.test.comparators) == 1
and isinstance(node.test.comparators[0], ast.Constant)
and node.test.comparators[0].value is None
for node in ast.walk(endpoint)
)
print({
"endpoint_calls_get_user_with_id": calls_helper,
"helper_raises_NoResultFound": raises_no_result,
"endpoint_catches_NoResultFound": has_try_except_no_result,
"endpoint_has_none_guard": has_none_guard,
"endpoint_mentions_NoResultFound": "NoResultFound" in router_text,
"database_source_mentions_NoResultFound": "NoResultFound" in db_text,
})
PYRepository: Agenta-AI/agenta
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -e
rg -n -S -g '*.py' 'add_exception_handler|exception_handlers|NoResultFound|@.*exception' api/oss/src api/oss/tests || true
rg -n -S -g '*.py' 'class APIRouter|def APIRouter' api/oss/src
cat -n api/oss/tests/legacy/old_tests/variants_user_profile_router/test_user_profile.py
rg -n -S 'py-run-tests|run-tests.py' README.md CONTRIBUTING.md api/Makefile api/pyproject.toml api 2>/dev/null | head -n 160 || true
python3 - <<'PY'
from pathlib import Path
import ast
router_path = Path("api/oss/src/routers/user_profile.py")
db_path = Path("api/oss/src/services/db_manager.py")
router_text = router_path.read_text()
db_text = db_path.read_text()
router = ast.parse(router_text)
db = ast.parse(db_text)
def find_function(tree, name):
for node in ast.walk(tree):
if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)) and node.name == name:
return node
raise LookupError(name)
endpoint = find_function(router, "user_profile")
helper = find_function(db, "get_user_with_id")
print({
"endpoint_calls_get_user_with_id": any(
isinstance(n, ast.Call) and isinstance(n.func, ast.Attribute)
and n.func.attr == "get_user_with_id" for n in ast.walk(endpoint)
),
"helper_raises_NoResultFound": any(
isinstance(n, ast.Raise) and isinstance(n.exc, ast.Call)
and isinstance(n.exc.func, ast.Name)
and n.exc.func.id == "NoResultFound" for n in ast.walk(helper)
),
"endpoint_catches_NoResultFound": any(
isinstance(n, ast.Try)
and any(isinstance(h.type, ast.Name) and h.type.id == "NoResultFound"
for h in n.handlers)
for n in ast.walk(endpoint)
),
})
PYRepository: Agenta-AI/agenta
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- custom APIRouter ---'
cat -n api/oss/src/utils/common.py | sed -n '1,90p'
printf '%s\n' '--- exception utility definitions ---'
cat -n api/oss/src/utils/exceptions.py | sed -n '1,220p'
printf '%s\n' '--- application construction and handlers ---'
rg -l -S -g '*.py' 'FastAPI\(|add_exception_handler|exception_handlers' api/oss/src | while read -r f; do
echo "--- $f"
rg -n -S 'FastAPI\(|add_exception_handler|exception_handlers' "$f"
done
printf '%s\n' '--- exact test command references ---'
rg -n -S -g '!**/.venv/**' -g '!**/node_modules/**' 'py-run-tests|run-tests.py' . | head -n 120 || true
printf '%s\n' '--- route AST exception-flow verifier ---'
python3 - <<'PY'
from pathlib import Path
import ast
router_path = Path("api/oss/src/routers/user_profile.py")
db_path = Path("api/oss/src/services/db_manager.py")
router = ast.parse(router_path.read_text())
db = ast.parse(db_path.read_text())
def fn(tree, name):
return next(
n for n in ast.walk(tree)
if isinstance(n, (ast.FunctionDef, ast.AsyncFunctionDef)) and n.name == name
)
endpoint = fn(router, "user_profile")
helper = fn(db, "get_user_with_id")
def has_call(node, attr):
return any(
isinstance(n, ast.Call) and isinstance(n.func, ast.Attribute)
and n.func.attr == attr for n in ast.walk(node)
)
def raises_name(node, name):
return any(
isinstance(n, ast.Raise)
and isinstance(n.exc, ast.Call)
and isinstance(n.exc.func, ast.Name)
and n.exc.func.id == name for n in ast.walk(node)
)
def catches_name(node, name):
return any(
isinstance(n, ast.Try)
and any(
isinstance(h.type, ast.Name) and h.type.id == name
for h in n.handlers
)
for n in ast.walk(node)
)
print("endpoint_calls_helper:", has_call(endpoint, "get_user_with_id"))
print("helper_raises_NoResultFound:", raises_name(helper, "NoResultFound"))
print("endpoint_catches_NoResultFound:", catches_name(endpoint, "NoResultFound"))
PYRepository: Agenta-AI/agenta
Length of output: 22575
🏁 Script executed:
#!/bin/bash
set -e
cat -n api/oss/src/utils/common.py | sed -n '1,90p'
cat -n api/oss/src/utils/exceptions.py | sed -n '1,220p'
rg -l -S -g '*.py' 'FastAPI\(|add_exception_handler|exception_handlers' api/oss/src | while read -r f; do
echo "--- $f"
rg -n -S 'FastAPI\(|add_exception_handler|exception_handlers' "$f"
done
rg -n -S -g '!**/.venv/**' -g '!**/node_modules/**' 'py-run-tests|run-tests.py' . | head -n 120 || true
python3 - <<'PY'
from pathlib import Path
import ast
router = ast.parse(Path("api/oss/src/routers/user_profile.py").read_text())
db = ast.parse(Path("api/oss/src/services/db_manager.py").read_text())
def fn(tree, name):
return next(n for n in ast.walk(tree)
if isinstance(n, (ast.FunctionDef, ast.AsyncFunctionDef))
and n.name == name)
endpoint = fn(router, "user_profile")
helper = fn(db, "get_user_with_id")
def has_call(node, attr):
return any(isinstance(n, ast.Call)
and isinstance(n.func, ast.Attribute)
and n.func.attr == attr for n in ast.walk(node))
def raises_name(node, name):
return any(isinstance(n, ast.Raise)
and isinstance(n.exc, ast.Call)
and isinstance(n.exc.func, ast.Name)
and n.exc.func.id == name for n in ast.walk(node))
def catches_name(node, name):
return any(isinstance(n, ast.Try)
and any(isinstance(h.type, ast.Name) and h.type.id == name
for h in n.handlers)
for n in ast.walk(node))
print("endpoint_calls_helper:", has_call(endpoint, "get_user_with_id"))
print("helper_raises_NoResultFound:", raises_name(helper, "NoResultFound"))
print("endpoint_catches_NoResultFound:", catches_name(endpoint, "NoResultFound"))
PYRepository: Agenta-AI/agenta
Length of output: 22386
Handle NoResultFound around get_user_with_id().
get_user_with_id() raises NoResultFound for a missing user, so the user is None branch is unreachable. Catch the exception and raise the 404 HTTPException, or change the helper to return None. Add a regression test and run cd api && py-run-tests.
Source: Coding guidelines
…ser_profile assert statements are stripped when Python runs with -O (optimize). This caused silent failures where None leaked into downstream code. - db_manager.py: assert workspace_id → if + raise ValueError - user_profile.py: assert user_id → if + raise HTTPException Verified with custom test script under both python and python -O. No behavior change under normal execution — only makes the code safe under python -O.
shafeeq27edu-ai
force-pushed
the
fix/assert-to-exceptions-db-manager
branch
from
August 24, 2026 15:31
5010985 to
9d188fe
Compare
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.
Description
This PR replaces
assertstatements with explicit exceptions in production API code.assertis stripped when Python runs with-O(optimize), causing silent failures whereNoneleaks into downstream functions.Summary
What changed? Replaced
assert workspace_id is not Nonewithif workspace_id is None: raise ValueError(...)indb_manager.py, andassert user_id is not Nonewithif user_id is None: raise HTTPException(...)inuser_profile.py.Why was this change needed?
assertstatements are completely removed by Python's-Ooptimization flag. In production deployments that usepython -O, these guards vanish silently. The variables being asserted (workspace_id, user_id) are then passed asNoneto downstream database queries, causing 500 errors or data corruption.What problem does it solve? Prevents silent failures under optimized Python execution. Makes validation explicit and guaranteed to run regardless of Python flags.
How the change addresses the root cause: By converting
asserttoif + raise, the validation becomes real runtime code that survivespython -O. The exception types match the context:ValueErrorfor service-layer functions,HTTPExceptionfor router handlers.Testing
Verified locally
assertlines exist at the reported locations indb_manager.pyanduser_profile.pypython -Ostrips assert statements (no output, no error)if + raisesurvivespython -Oand raises correctlyruff checkon both files — passes with no errorsruff formaton both files — no changes neededAdded or updated tests
N/A — This is a defensive coding fix with no behavior change under normal execution. Existing validation tests cover the functional path.
QA follow-up
N/A — The fix is self-contained and verified by Python's
-Obehavior.Demo
Before Fix (assert stripped by python -O)
After Fix