Explain model recommendation badges#5
Conversation
|
Warning Review limit reached
More reviews will be available in 48 minutes and 28 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughAdds four computed display fields ( ChangesModel Display Label and Reason Fields
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 09a3ce22ad
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| switch modelSectionKey(m) { | ||
| case "recommended": | ||
| return "Recommended" | ||
| case "interesting": | ||
| return "Interesting" |
There was a problem hiding this comment.
Preserve candidate status for catalog results
For non-preset catalog/search results, annotateModelForRole leaves Section empty and sets Policy to candidate; calling modelSectionKey here falls through to that helper's default "interesting", so every ordinary model in the full browser and Telegram search now renders an Interesting badge while the new template suppresses the raw candidate policy. This makes the recommendation bucket label appear on plain catalog candidates; distinguish an explicit section/source from the helper's bucket fallback before choosing the display status.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
internal/adminapi/model_display.go (1)
61-84: ⚡ Quick winRedundant if-else blocks in
modelPolicyLabel.Lines 62-65, 66-70, 71-76, and 80-84 all contain if-else blocks where both branches return the same value. For example:
case "manual_allow": if status == "Interesting" { return "Manual allow" } return "Manual allow"This pattern repeats for
manual_deny,free_unverified, andfree_blocked. The conditions check whether the status already conveys the policy (e.g., manual_allow + Interesting), suggesting deduplication was intended, but both branches return the same label.Either:
- The if-branch should return
""to suppress duplicate labels (matching the deduplication for "recommended" on line 59), or- The if conditions are vestigial and should be removed for clarity.
Since tests only verify suppression for "recommended" policy and expect other policy labels to be shown (e.g.,
PolicyLabel="Free unverified"inadminapi_test.go:433), the conditions appear unnecessary.♻️ Proposed fix to remove redundant conditions
case "manual_allow": - if status == "Interesting" { - return "Manual allow" - } return "Manual allow" case "manual_deny": - if status == "Blocked" { - return "Manual deny" - } return "Manual deny" case "free_unverified": - if status == "Untested" { - return "Free unverified" - } return "Free unverified" case "free_verified": return "Free verified" case "free_degraded": return "Free degraded" case "free_blocked": - if status == "Blocked" { - return "Free blocked" - } return "Free blocked"🤖 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 `@internal/adminapi/model_display.go` around lines 61 - 84, The modelPolicyLabel function contains redundant if-else blocks in the cases for manual_allow, manual_deny, free_unverified, and free_blocked where both the if-branch and else-branch return the same value. Since tests expect these policy labels to always be displayed and both branches are identical, remove the unnecessary conditional checks. For each of these four case statements, replace the if-else block with a single direct return statement that returns the label value immediately, matching the simpler pattern already used in the free_verified and free_degraded cases.
🤖 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.
Nitpick comments:
In `@internal/adminapi/model_display.go`:
- Around line 61-84: The modelPolicyLabel function contains redundant if-else
blocks in the cases for manual_allow, manual_deny, free_unverified, and
free_blocked where both the if-branch and else-branch return the same value.
Since tests expect these policy labels to always be displayed and both branches
are identical, remove the unnecessary conditional checks. For each of these four
case statements, replace the if-else block with a single direct return statement
that returns the label value immediately, matching the simpler pattern already
used in the free_verified and free_degraded cases.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c56b8343-7d69-4132-b4ab-fe3bd71f69a4
📒 Files selected for processing (7)
internal/adminapi/adminapi_test.gointernal/adminapi/handlers.gointernal/adminapi/model_display.gointernal/adminapi/recommend_test.gointernal/adminapi/templates/partials_models_row.htmlinternal/adminapi/templates/tg_admin.htmlinternal/adminapi/tgadmin.go
09a3ce2 to
1732c0d
Compare
Summary
Test Plan
Summary by CodeRabbit
New Features
Bug Fixes