fix: two 500s on feed-details and simulation-details read endpoints - #22
Merged
Conversation
Nutrient columns are Postgres NUMERIC and some feeds store the literal value
NaN. get_feed_details did float(Decimal('NaN')) -> nan and returned it in the
response dict; Starlette's JSONResponse serializes with allow_nan=False, so any
NaN raised "ValueError: Out of range float values are not JSON compliant: nan"
-> HTTP 500. On the Test DB this affected 81 feeds.
Pass the response through the existing ensure_json_safe() (NaN/inf -> 0.0), the
same helper the recommendation/evaluation responses already use.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…etails
Reports persist animal_inputs with the engine key `An_StatePhys`, but
GET /v1/animal/simulations/{report_id} validates cattle_info against CattleInfo,
whose field is `physiological_state` (required since the animal-category
feature). Building the model from the raw dict raised pydantic "Field required"
-> HTTP 500 for every report created after that feature shipped.
Map An_StatePhys -> physiological_state when assembling cattle_info so the model
validates and the endpoint returns 200.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughTwo service responses are normalized before return: feed details sanitize non-finite numeric values, and simulation details populate ChangesFeed response normalization
Simulation cattle mapping
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 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 |
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
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
End-to-end API testing against the live Test server surfaced two read endpoints returning HTTP 500. They looked like one issue but had two distinct root causes, each fixed here (one commit per fix).
1.
GET /v1/animal/feed-details/{id}— NaN JSON serializationNutrient columns are Postgres
NUMERICand some feeds store the literal valueNaN.get_feed_detailsdidfloat(Decimal('NaN'))→nanand returned it in the response dict; Starlette'sJSONResponseserializes withallow_nan=False, so anyNaNraised:Fix: pass the response through the existing
ensure_json_safe()(NaN/inf → 0.0) — the same helper the recommendation/evaluation responses already use.2.
GET /v1/animal/simulations/{report_id}— field-name mismatch (not NaN)Reports persist
animal_inputswith the engine keyAn_StatePhys, but the endpoint validatescattle_infoagainstCattleInfo, whose field isphysiological_state(required since the animal-category feature). Building the model from the raw dict raised pydanticField required→ 500 for every report created after that feature shipped.Fix: map
An_StatePhys→physiological_statewhen assemblingcattle_info.Data note (applied separately, not in this PR)
On the Test DB, 81 feeds held
NaNin nutrient columns (fd_cf,fd_nfe,fd_hemicellulose,fd_cellulose×81,fd_ee×1 = 325 cells); these were set toNULL(backed up first). That already cleared bug #1 on the live Test server; bug #2 needs this code deployed to clear live.Verification
app.mainimports cleanly.Scope
Two files, +19 / -3 lines:
services/diet_service.py,services/report_service.py. No dependency or schema changes.🤖 Generated with Claude Code
Summary by CodeRabbit