Repository navigation
feat(decisions): accept decisions as a model mode alongside evaluation - #45474
Conversation
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
|
kerry seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
|
| document=_ocr_health_check_document(model=model, custom_llm_provider=custom_llm_provider), | ||
| ), | ||
| "evaluation": lambda: litellm.adecisions( | ||
| "decisions": lambda: litellm.adecisions( |
There was a problem hiding this comment.
Real-provider proof is still pending. Repository rules require live-proxy checks of inferred modes, legacy configs, and Lens selection. Add that proof before merging
Rule Used: Flag if the PR has no proof of e2e testing (if e2e testing is applicable here. If it's like an AGENTS.md change or something then probably no need to have e2e test proof) that either doesn't exist, claims that the box that it was trying to run in doe... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Added live proof to the PR description: a real proxy against the TypeSafe API with no mocks, at merge base 1a42c5a and at c0bbbdb. It covers a cost-map decision model with no mode, an explicit mode: decisions deployment and a legacy mode: evaluation deployment, through /model_group/info, /health and a real /v1/systemone call
| mode = mode or default_health_check_mode( | ||
| requested_model=requested_model, model=model, custom_llm_provider=custom_llm_provider | ||
| ) | ||
| mode = canonical_model_mode(mode) |
There was a problem hiding this comment.
Parameter rebinding breaks repository rules
This reassigns the mode parameter, which AGENTS.md forbids. Use a separate final value for the normalized mode before merging
Context Used: AGENTS.md (source)
There was a problem hiding this comment.
Fixed in cb75948: canonical_model_mode now wraps the existing mode or default_health_check_mode(...) expression, so the parameter is not rebound a second time
…ding Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…odel_mode Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…tion_mode_to_decisions
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
Devin is archived and cannot be woken up. Please unarchive Devin if you want to continue using it. |
TLDR
Problem this solves:
mode: evaluation, which doesn't name the endpoint they serve/healthhas nodecisionshandlerevaluation, and new duplicate rows (follow-up PR) will usedecisions. This PR makes the code ready for those rowsHow it solves it:
model_prices_and_context_window.json, its backup andcost_tracking_cases.jsonare identical to mainlitellm/types/utils.pyaccepts both values:ahealth_checksends both values to the Decisions probe (handler_mode = "decisions" if is_decisions_model_mode(mode) else mode), and/health/test_connectionacceptsmode: decisionsisDecisionMode(Add Model provider lists, guardrail decision model picker) accepts bothModelInfoBase.modelists both valueslitellm-model-catalogModeenum getsDecisions.Catalog::parserejects the whole file on an unknown mode, so without this onedecisionsrow would make every Rust consumer of the cost map fail to load itci_cd/generate_model_prices_schema.pybuilds it from the modes rows actually use, so it picks updecisionswhen the new rows landUser Flow
Before: a deployment set to
model_info.mode: decisionsfails its health checktypesafe/jev-latestto config.yaml withmodel_info: {mode: decisions}Mode decisions not supportedAfter:
decisionsandevaluationbehave the sametypesafe/jev-latestwithmodel_info: {mode: decisions}model_info.modestill report"mode": "evaluation"in/model_group/info, same as todayPre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
uv run pytest tests/unit/<your_test_file>.py -v. Leave the suites (make test-unit-*,make test-unit) to CI: it finishes in ~15 minutes where a laptop takes an hour or more@greptileaito re-request a review after pushing changes)Screenshots / Proof of Fix
Live proxy at
06c6cd6eb5, real TypeSafe API, no mocks,LITELLM_LOCAL_MODEL_COST_MAP=True, no DB/model_group/infomode/healthtypesafe-jev-no-modeevaluationtypesafe-jev-decisionsdecisionstypesafe-jev-evaluationevaluationOn main the
typesafe-jev-decisionshealth check fails withMode decisions not supportedType
🆕 New Feature
Caveats (if any)
Low
model_info.mode: decisionsand then runs on a proxy older than this PR still fails/healthwithMode decisions not supported. Only configs that opt in can hit thisdecisionsrows will returnMode decisions not supportedfrom/healthon proxies older than this PR. Real/v1/decisionsand/v1/systemonecalls to them still work, sincemodeonly picks the health probemode == "evaluation"exactly, so it needs its own change before it listsdecisionsmodels. Its bundled cost map snapshot also parses with a pinnedlitellm-model-catalog, so refreshing that snapshot afterdecisionsrows land needs the pin bumped past this PRevaluation, so a model added through the UI keeps working if the proxy is rolled backrole: "evaluation"is a different concept and is unchangedFinal Attestation