cleaned up gen ai services#123
Conversation
|
Warning Review limit reached
Next review available in: 28 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughBoth services switch to LangChain structured output with local Pydantic schemas, update LLM provider settings, and adjust tests to mock structured runnable responses instead of raw JSON content. ChangesHelp Service Structured Output
Recipe Service Structured Output
Estimated code review effort: 3 (Moderate) | ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 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: 3
🤖 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 `@services/py-recipe-service/main.py`:
- Around line 37-49: Keep LocalRecipeInput aligned with the public /ai/recipes
contract by removing nutrients from the schema or making sure it is not included
in the response path; update the RecipeInput.from_dict(...) usage in the
recipe-building flow so only title, ingredients, instructions, and portions are
serialized into the returned object. Check the LocalRecipeInput and
RecipeInput.from_dict symbols together to ensure the downstream response shape
stays unchanged unless the API contract is intentionally updated.
- Around line 208-216: The recipe-service test double is still mocking direct
LLM invocation, but `create_recipe()` now uses
`llm.with_structured_output(RecipeListWrapper).ainvoke(...)`. Update the test in
`test_recipe_service.py` so `mock_llm.with_structured_output()` returns a
runnable/mock that exposes `ainvoke`, and move the `ainvoke`
expectation/assertion to that returned object instead of `mock_llm` directly.
Use the `create_recipe` flow and `RecipeListWrapper` path to locate the change.
- Around line 155-157: The Gemini provider setup in init_chat_model is passing
an unsupported thinking_level kwarg through kwargs, which can break startup with
langchain-google-genai==2.1.12. Remove the thinking_level assignment in
main.py’s model initialization path, or update the dependency to a version that
supports it, and make sure the google_genai branch only forwards accepted
parameters.
🪄 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 Plus
Run ID: 29f99c9f-899d-4705-9071-2295d0bdaef6
📒 Files selected for processing (2)
services/py-help-service/main.pyservices/py-recipe-service/main.py
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
services/py-help-service/tests/test_help_service.py (1)
41-41: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider asserting
with_structured_outputis called withLocalHelpResponse.The mock accepts any argument to
with_structured_output, so the test wouldn't catch a regression where the endpoint passes the wrong schema class. Could addmock_llm.with_structured_output.assert_called_once_with(LocalHelpResponse).Also applies to: 78-78
🤖 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 `@services/py-help-service/tests/test_help_service.py` at line 41, The tests around the help service should verify that the LLM wrapper is configured with the correct response schema, not just any schema. Update the relevant test cases that use mock_llm.with_structured_output and assert it is called once with LocalHelpResponse so a wrong schema class in the endpoint would fail the test.
🤖 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 `@services/py-recipe-service/main.py`:
- Line 7: Handle the omitted nutrients case before constructing RecipeInput: in
the code path that calls r.model_dump() and RecipeInput.from_dict(), make sure a
None nutrients value is treated the same as a missing key so
RecipeNutrients.from_dict is never called with None. Update the relevant
conversion logic around RecipeInput.from_dict and the RecipeNutrients parsing
branch to skip or remove nutrients when it is None, preserving existing behavior
for present nutrient data.
---
Nitpick comments:
In `@services/py-help-service/tests/test_help_service.py`:
- Line 41: The tests around the help service should verify that the LLM wrapper
is configured with the correct response schema, not just any schema. Update the
relevant test cases that use mock_llm.with_structured_output and assert it is
called once with LocalHelpResponse so a wrong schema class in the endpoint would
fail the test.
🪄 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 Plus
Run ID: c26af8af-befb-4b60-a10a-fed94f962d35
📒 Files selected for processing (6)
services/py-help-service/main.pyservices/py-help-service/requirements.txtservices/py-help-service/tests/test_help_service.pyservices/py-recipe-service/main.pyservices/py-recipe-service/requirements.txtservices/py-recipe-service/tests/test_recipe_service.py
🚧 Files skipped from review as they are similar to previous changes (1)
- services/py-help-service/main.py
| import time | ||
| import traceback | ||
| from typing import Any | ||
| from typing import Any, List, Optional |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
ast-grep run --pattern $'def generate_recipes($$$) {
$$$
}' --lang python services/py-recipe-service/main.py
rg -n -B3 -A10 'RecipeInput\.from_dict|\.nutrients' services/py-recipe-service/main.pyRepository: AET-DevOps26/team-devsecops
Length of output: 984
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- main.py lines 1-120 ---'
sed -n '1,120p' services/py-recipe-service/main.py
echo
echo '--- main.py lines 170-235 ---'
sed -n '170,235p' services/py-recipe-service/main.py
echo
echo '--- nutrients references ---'
rg -n 'nutrients|LocalRecipeInput|RecipeInput\.from_dict|model_dump\(\)' services/py-recipe-service/main.pyRepository: AET-DevOps26/team-devsecops
Length of output: 6215
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the model definitions and conversion logic around the suspected None path.
sed -n '1,120p' services/py-recipe-service/main.py
printf '\n---\n'
sed -n '170,235p' services/py-recipe-service/main.py
printf '\n---\n'
rg -n 'nutrients|LocalRecipeInput|RecipeInput\.from_dict|model_dump\(\)' services/py-recipe-service/main.pyRepository: AET-DevOps26/team-devsecops
Length of output: 6136
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the structured-output schema and any conversion code that handles nutrients.
rg -n -A8 -B8 'class LocalRecipeInput|class LocalRecipeNutrients|with_structured_output|RecipeInput\.from_dict|nutrients' services/py-recipe-service/main.pyRepository: AET-DevOps26/team-devsecops
Length of output: 2048
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the RecipeInput type and its from_dict implementation.
rg -n -A20 -B10 'class RecipeInput|def from_dict|RecipeInput\(' .Repository: AET-DevOps26/team-devsecops
Length of output: 50384
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the exact RecipeInput deserialization path used by services/py-recipe-service/main.py.
recipe_file='services/py-recipe-service/client/cooking_assistant_gen_ai_services_api_internal_client/models/recipe_input.py'
wc -l "$recipe_file"
printf '\n---\n'
sed -n '1,220p' "$recipe_file"Repository: AET-DevOps26/team-devsecops
Length of output: 3015
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Narrowly inspect the deserializer and its handling of optional nutrients.
python3 - <<'PY'
from pathlib import Path
p = Path("services/py-recipe-service/client/cooking_assistant_gen_ai_services_api_internal_client/models/recipe_input.py")
text = p.read_text()
for needle in ["def from_dict", "nutrients", "RecipeNutrients.from_dict", "if nutrients is not UNSET", "if nutrients is not None"]:
idx = text.find(needle)
print(f"\n--- {needle} ---")
if idx == -1:
print("not found")
continue
start = max(0, text.rfind("\n", 0, idx-200))
end = min(len(text), idx+700)
print(text[start:end])
PYRepository: AET-DevOps26/team-devsecops
Length of output: 3580
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check whether model_dump() includes None by default in the available Pydantic version.
python3 - <<'PY'
from pydantic import BaseModel
from typing import Optional
class Child(BaseModel):
x: int
class Parent(BaseModel):
child: Optional[Child] = None
print(Parent().model_dump())
print(Parent(child=None).model_dump())
PYRepository: AET-DevOps26/team-devsecops
Length of output: 197
Handle nutrients=None before building RecipeInput. r.model_dump() emits "nutrients": None for omitted optional fields, but RecipeInput.from_dict() only treats a missing key as absent; None reaches RecipeNutrients.from_dict(None) and raises.
🤖 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 `@services/py-recipe-service/main.py` at line 7, Handle the omitted nutrients
case before constructing RecipeInput: in the code path that calls r.model_dump()
and RecipeInput.from_dict(), make sure a None nutrients value is treated the
same as a missing key so RecipeNutrients.from_dict is never called with None.
Update the relevant conversion logic around RecipeInput.from_dict and the
RecipeNutrients parsing branch to skip or remove nutrients when it is None,
preserving existing behavior for present nutrient data.
1 participant
Summary
Cleaned up LLM config by using
with_structured_outputpattern.Fixed: Only recipe title was sent to LLM before.
Ran experiments:
Logos needs more than 60s if
reasoning_effortis set tomediumorhigh.For google_genai the
thinking_levelis also set tolowas it appears to be balancing the tradeoff between inference time and quality/reliability best.Kept generated recipes at 3 as more recipes cause longer inference time especially for Logos.
Type of change
API changes
api/openapi.yamlupdated andapi/scripts/gen-all.shre-runDefinition of Done
Summary by CodeRabbit