Correct the denominator in the model-agreement endpoint's API documentation - #1394
Conversation
…text Six help_text strings on ModelAgreementSerializer still named verified_with_prediction_count as the denominator for agreed_any_rank_pct, agreed_coarser_rank_pct and all four Wilson bounds. The implementation divides every one of them by comparable_count, and the two differ whenever a verification carries no taxon — a comment-only identification has a machine prediction but nothing to compare it against. A consumer reading the schema and recomputing a percentage from the counts would therefore get a different number than the endpoint reports. The class docstring already described the behaviour correctly; only the per-field strings had drifted. The same confusion was fixed on the frontend in #1308. Two test additions pin the corrected claims, both previously unpinned: agreed_any_rank_pct divides by comparable_count, and the Wilson bounds go null on an empty comparable set even while verified_with_prediction_count is non-zero. Co-Authored-By: Claude <noreply@anthropic.com>
✅ Deploy Preview for antenna-preview canceled.
|
✅ Deploy Preview for antenna-ssec canceled.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe change updates model-agreement help text to use ChangesModel agreement metrics
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This change corrects model-agreement documentation and adds tests without changing endpoint behavior, so no actionable merge-blocking risk remains after normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Description checkExplanation The description explains the problem, affected fields, consumer impact, test coverage, validation steps, and deployment impact. It omits some template headings, but it is sufficiently complete and directly relevant. ✨ 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 |
There was a problem hiding this comment.
🟢 Approval recommended
The changes are narrowly scoped to documentation help text plus targeted tests that validate the documented denominator/CI behavior without altering endpoint logic.
Pull request overview
This PR fixes drift in the ModelAgreement API schema documentation so that agreement percentages and Wilson confidence interval fields correctly describe comparable_count (not verified_with_prediction_count) as the denominator/guard condition, aligning the OpenAPI/browsable-API help text with the endpoint’s actual behavior.
Changes:
- Update
ModelAgreementSerializerfieldhelp_textto referencecomparable_countforagreed_*_pctand CI nullability. - Add tests that (1) assert
agreed_any_rank_pctusescomparable_countand (2) verify CI bounds returnNonewhencomparable_count == 0even if predictions exist.
File summaries
| File | Description |
|---|---|
ami/main/api/serializers.py |
Corrects per-field help text to name comparable_count as the denominator / CI-null condition. |
ami/main/tests.py |
Adds coverage ensuring the agreement pct denominator and CI nullability match comparable_count semantics. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Summary
The API documentation for the model-agreement endpoint named the wrong denominator. Six of the field descriptions said the agreement percentages and confidence intervals were calculated over
verified_with_prediction_count, when the endpoint actually divides them bycomparable_count. Those two numbers are not the same: a verification that carries no taxon — someone leaving a comment rather than naming a species — has a machine prediction but nothing to compare it against, so it counts toward the first and not the second. Anyone reading the schema and recomputing a percentage from the counts we return would get a different answer than the endpoint gives them.Nothing about the endpoint's behaviour changes here. The numbers it returns were always calculated against
comparable_count, and the serializer's own class docstring already said so. Only the per-field descriptions had drifted away from it, and those are what a consumer actually sees in the browsable API and the generated OpenAPI schema.The same confusion had been shipped in the frontend tooltips, and was corrected there in #1308.
List of Changes
agreed_any_rank_pcthelp textagreed_coarser_rank_pcthelp textagreed_exact_ci_low/_highandagreed_any_rank_ci_low/_highhelp textTestModelAgreementForProjectgains an assertion that the any-rank percentage divides bycomparable_count, plus a case where every verification is comment-only, soverified_with_prediction_countis non-zero whilecomparable_countis zero and all four bounds come back nullTest plan
TestModelAgreementForProject— 5 tests green (4 existing plus the new confidence-interval case).-k Agreement -k Stats— 25 tests green.flake8included.wilson_interval(ami/utils/stats.py:33) returnsNonewhen itstotalargument is zero, andmodel_agreement_for_projectpassescomparableas that argument. The percentages are built by_pct(..., comparable)(ami/main/models_future/occurrence.py:294-295, 325).Summary by CodeRabbit
Bug Fixes
Tests