mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-20 00:11:50 +00:00
feat(policy_engine): bound priority to int32 and expose it in the Admin UI
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
parent
1b1f6ada46
commit
669a66499c
13 changed files with 215 additions and 8 deletions
|
|
@ -33932,6 +33932,8 @@
|
|||
"priority": {
|
||||
"anyOf": [
|
||||
{
|
||||
"maximum": 2147483647.0,
|
||||
"minimum": -2147483648.0,
|
||||
"type": "integer"
|
||||
},
|
||||
{
|
||||
|
|
@ -36089,6 +36091,8 @@
|
|||
"priority": {
|
||||
"anyOf": [
|
||||
{
|
||||
"maximum": 2147483647.0,
|
||||
"minimum": -2147483648.0,
|
||||
"type": "integer"
|
||||
},
|
||||
{
|
||||
|
|
|
|||
|
|
@ -290,6 +290,8 @@ class PolicyAttachment(BaseModel):
|
|||
)
|
||||
priority: int | None = Field(
|
||||
default=None,
|
||||
ge=-2147483648,
|
||||
le=2147483647,
|
||||
description="Explicit execution order, lower runs first. Prioritised attachments run before those without one.",
|
||||
)
|
||||
|
||||
|
|
|
|||
|
|
@ -307,6 +307,8 @@ class PolicyAttachmentCreateRequest(BaseModel):
|
|||
)
|
||||
priority: int | None = Field(
|
||||
default=None,
|
||||
ge=-2147483648,
|
||||
le=2147483647,
|
||||
description="Explicit execution order, lower runs first. Prioritised attachments run before those without one.",
|
||||
)
|
||||
|
||||
|
|
|
|||
|
|
@ -189,6 +189,37 @@ class TestGetAttachedPolicies:
|
|||
|
||||
assert registry.get_attached_policies(context) == ["model-policy", "team-policy"]
|
||||
|
||||
def test_equal_priority_attachments_fall_back_to_scope_tier_order(self):
|
||||
registry = AttachmentRegistry()
|
||||
registry.load_attachments(
|
||||
[
|
||||
{"policy": "model-policy", "models": ["gpt-4"], "priority": 1},
|
||||
{"policy": "tag-policy", "tags": ["prod"], "priority": 1},
|
||||
{"policy": "global-policy", "scope": "*", "priority": 1},
|
||||
]
|
||||
)
|
||||
|
||||
context = PolicyMatchContext(model="gpt-4", tags=["prod"])
|
||||
|
||||
assert registry.get_attached_policies(context) == ["global-policy", "tag-policy", "model-policy"]
|
||||
|
||||
def test_duplicate_policy_uses_highest_priority_attachment(self):
|
||||
registry = AttachmentRegistry()
|
||||
registry.load_attachments(
|
||||
[
|
||||
{"policy": "shared-policy", "scope": "*"},
|
||||
{"policy": "global-policy", "scope": "*"},
|
||||
{"policy": "shared-policy", "models": ["gpt-4"], "priority": 0},
|
||||
]
|
||||
)
|
||||
|
||||
context = PolicyMatchContext(model="gpt-4")
|
||||
|
||||
assert registry.get_attached_policies_with_reasons(context) == [
|
||||
{"policy_name": "shared-policy", "matched_via": "model:gpt-4"},
|
||||
{"policy_name": "global-policy", "matched_via": "scope:*"},
|
||||
]
|
||||
|
||||
def test_combined_team_and_model_attachment_uses_model_specificity(self):
|
||||
registry = AttachmentRegistry()
|
||||
registry.load_attachments(
|
||||
|
|
|
|||
|
|
@ -0,0 +1,15 @@
|
|||
import pytest
|
||||
from pydantic import ValidationError
|
||||
|
||||
from litellm.types.proxy.policy_engine.policy_types import PolicyAttachment
|
||||
|
||||
|
||||
@pytest.mark.parametrize("priority", [-2147483648, 2147483647])
|
||||
def test_policy_attachment_accepts_int32_priority(priority: int):
|
||||
assert PolicyAttachment(policy="p", priority=priority).priority == priority
|
||||
|
||||
|
||||
@pytest.mark.parametrize("priority", [-2147483649, 2147483648])
|
||||
def test_policy_attachment_rejects_priority_outside_int32(priority: int):
|
||||
with pytest.raises(ValidationError):
|
||||
PolicyAttachment(policy="p", priority=priority)
|
||||
|
|
@ -3,8 +3,10 @@ Tests for pipeline field on policy CRUD types (resolver_types.py).
|
|||
"""
|
||||
|
||||
import pytest
|
||||
from pydantic import ValidationError
|
||||
|
||||
from litellm.types.proxy.policy_engine.resolver_types import (
|
||||
PolicyAttachmentCreateRequest,
|
||||
PolicyCreateRequest,
|
||||
PolicyDBResponse,
|
||||
PolicyUpdateRequest,
|
||||
|
|
@ -100,3 +102,14 @@ def test_policy_create_request_roundtrip():
|
|||
dumped = req.model_dump()
|
||||
restored = PolicyCreateRequest(**dumped)
|
||||
assert restored.pipeline == pipeline_data
|
||||
|
||||
|
||||
@pytest.mark.parametrize("priority", [-2147483648, 2147483647])
|
||||
def test_policy_attachment_create_request_accepts_int32_priority(priority: int):
|
||||
assert PolicyAttachmentCreateRequest(policy_name="p", priority=priority).priority == priority
|
||||
|
||||
|
||||
@pytest.mark.parametrize("priority", [-2147483649, 2147483648])
|
||||
def test_policy_attachment_create_request_rejects_priority_outside_int32(priority: int):
|
||||
with pytest.raises(ValidationError):
|
||||
PolicyAttachmentCreateRequest(policy_name="p", priority=priority)
|
||||
|
|
|
|||
|
|
@ -45,9 +45,26 @@ describe("AttachmentTable", () => {
|
|||
expect(screen.getByText("Keys")).toBeInTheDocument();
|
||||
expect(screen.getByText("Models")).toBeInTheDocument();
|
||||
expect(screen.getByText("Tags")).toBeInTheDocument();
|
||||
expect(screen.getByText("Priority")).toBeInTheDocument();
|
||||
expect(screen.getByText("Created At")).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("should show the priority and a dash for attachments without one", () => {
|
||||
const attachments = [
|
||||
makeAttachment({ attachment_id: "att-prio0001", policy_name: "prioritized", priority: 5 }),
|
||||
makeAttachment({ attachment_id: "att-prio0002", policy_name: "unprioritized" }),
|
||||
];
|
||||
renderWithProviders(<AttachmentTable {...defaultProps} attachments={attachments} />);
|
||||
const rows = screen.getAllByRole("row").slice(1);
|
||||
const prioritizedRow = rows.find((row) => within(row).queryByText("prioritized"));
|
||||
const unprioritizedRow = rows.find((row) => within(row).queryByText("unprioritized"));
|
||||
expect(within(prioritizedRow!).getByText("5")).toBeInTheDocument();
|
||||
expect(within(unprioritizedRow!).queryByText("5")).not.toBeInTheDocument();
|
||||
expect(within(unprioritizedRow!).getAllByText("-")).toHaveLength(
|
||||
within(prioritizedRow!).getAllByText("-").length + 1,
|
||||
);
|
||||
});
|
||||
|
||||
it("should show skeleton rows when isLoading is true", () => {
|
||||
renderWithProviders(<AttachmentTable {...defaultProps} isLoading />);
|
||||
expect(screen.getAllByTestId("skeleton-row").length).toBeGreaterThan(0);
|
||||
|
|
|
|||
|
|
@ -167,6 +167,20 @@ export const getAttachmentTableColumns = ({
|
|||
enableSorting: false,
|
||||
cell: ({ row }) => <ChipList values={row.original.tags ?? []} />,
|
||||
},
|
||||
{
|
||||
id: "priority",
|
||||
accessorFn: (row) => row.priority ?? Number.POSITIVE_INFINITY,
|
||||
meta: { title: "Priority" },
|
||||
header: ({ column }) => <DataTableSortHeader column={column} title="Priority" />,
|
||||
size: 100,
|
||||
enableSorting: true,
|
||||
cell: ({ row }) =>
|
||||
row.original.priority == null ? (
|
||||
<span className="text-muted-foreground">-</span>
|
||||
) : (
|
||||
<span className="font-mono text-xs">{row.original.priority}</span>
|
||||
),
|
||||
},
|
||||
{
|
||||
id: "created_at",
|
||||
accessorFn: (row) => row.created_at ?? "",
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
import React from "react";
|
||||
import { screen, waitFor } from "@testing-library/react";
|
||||
import { fireEvent, screen, waitFor } from "@testing-library/react";
|
||||
import userEvent from "@testing-library/user-event";
|
||||
import { renderWithProviders } from "@/../tests/test-utils";
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
|
@ -180,6 +180,61 @@ describe("AddAttachmentForm", () => {
|
|||
expect(screen.queryByText(TEAMS_ERROR)).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
const selectPolicy = async (user: UserEvent, policyName: string) => {
|
||||
await screen.findByText("Create Policy Attachment");
|
||||
const input = screen.getByLabelText("Policies");
|
||||
await user.click(input);
|
||||
await user.type(input, `${policyName}{Enter}`);
|
||||
};
|
||||
|
||||
const setPriority = (value: string) => {
|
||||
fireEvent.change(screen.getByLabelText("Priority"), { target: { value } });
|
||||
};
|
||||
|
||||
const submit = async (user: UserEvent) => {
|
||||
await user.click(screen.getByRole("button", { name: /create attachment/i }));
|
||||
};
|
||||
|
||||
it("sends the entered priority with the attachment", async () => {
|
||||
const user = userEvent.setup();
|
||||
const createAttachment = vi.fn().mockResolvedValue({});
|
||||
renderWithProviders(<AddAttachmentForm {...defaultProps} createAttachment={createAttachment} />);
|
||||
await selectPolicy(user, "policy-alpha");
|
||||
setPriority("10");
|
||||
await submit(user);
|
||||
await waitFor(() => expect(createAttachment).toHaveBeenCalledTimes(1));
|
||||
expect(createAttachment).toHaveBeenCalledWith("test-token", {
|
||||
policy_name: "policy-alpha",
|
||||
scope: "*",
|
||||
priority: 10,
|
||||
});
|
||||
});
|
||||
|
||||
it("omits priority from the attachment when the field is left blank", async () => {
|
||||
const user = userEvent.setup();
|
||||
const createAttachment = vi.fn().mockResolvedValue({});
|
||||
renderWithProviders(<AddAttachmentForm {...defaultProps} createAttachment={createAttachment} />);
|
||||
await selectPolicy(user, "policy-alpha");
|
||||
await submit(user);
|
||||
await waitFor(() => expect(createAttachment).toHaveBeenCalledTimes(1));
|
||||
expect(createAttachment).toHaveBeenCalledWith("test-token", { policy_name: "policy-alpha", scope: "*" });
|
||||
});
|
||||
|
||||
it.each([
|
||||
["2147483648", /at most 2147483647/i],
|
||||
["-2147483649", /at least -2147483648/i],
|
||||
["1.5", /whole number/i],
|
||||
])("blocks submit with a field error when priority is %s", async (value, error) => {
|
||||
const user = userEvent.setup();
|
||||
const createAttachment = vi.fn();
|
||||
renderWithProviders(<AddAttachmentForm {...defaultProps} createAttachment={createAttachment} />);
|
||||
await selectPolicy(user, "policy-alpha");
|
||||
setPriority(value);
|
||||
await submit(user);
|
||||
expect(await screen.findByText(error)).toBeInTheDocument();
|
||||
expect(createAttachment).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("defers to the backend (does not flag) when the team list failed to load", async () => {
|
||||
const user = userEvent.setup();
|
||||
vi.mocked(networking.teamListCall).mockRejectedValue(new Error("boom"));
|
||||
|
|
|
|||
|
|
@ -8,6 +8,7 @@ import useAuthorized from "@/app/(dashboard)/hooks/useAuthorized";
|
|||
import { FieldGroup, FieldLabel, FieldTitle } from "@/components/ui/field";
|
||||
import { FormField } from "@/components/shared/form/FormField";
|
||||
import { Button } from "@/components/ui/button";
|
||||
import { Input } from "@/components/ui/input";
|
||||
import { RadioGroup, RadioGroupItem } from "@/components/ui/radio-group";
|
||||
import { Separator } from "@/components/ui/separator";
|
||||
import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from "@/components/ui/tooltip";
|
||||
|
|
@ -36,6 +37,7 @@ interface AttachmentFormValues {
|
|||
keys: string[];
|
||||
models: string[];
|
||||
tags: string[];
|
||||
priority: number | null;
|
||||
}
|
||||
|
||||
const EMPTY_VALUES: AttachmentFormValues = {
|
||||
|
|
@ -44,14 +46,24 @@ const EMPTY_VALUES: AttachmentFormValues = {
|
|||
keys: [],
|
||||
models: [],
|
||||
tags: [],
|
||||
priority: null,
|
||||
};
|
||||
|
||||
const INT32_MIN = -2147483648;
|
||||
const INT32_MAX = 2147483647;
|
||||
|
||||
const attachmentShape = {
|
||||
policy_names: z.array(z.string()).min(1, "Please select at least one policy"),
|
||||
teams: z.array(z.string()),
|
||||
keys: z.array(z.string()),
|
||||
models: z.array(z.string()),
|
||||
tags: z.array(z.string()),
|
||||
priority: z
|
||||
.number({ error: "Priority must be a whole number" })
|
||||
.int("Priority must be a whole number")
|
||||
.min(INT32_MIN, `Priority must be at least ${INT32_MIN}`)
|
||||
.max(INT32_MAX, `Priority must be at most ${INT32_MAX}`)
|
||||
.nullable(),
|
||||
};
|
||||
|
||||
const buildAttachmentSchema = (scopeType: ScopeType, teamsLoaded: boolean, availableTeams: string[]) =>
|
||||
|
|
@ -419,6 +431,28 @@ const AddAttachmentForm: React.FC<AddAttachmentFormProps> = ({
|
|||
</FormField>
|
||||
</>
|
||||
)}
|
||||
|
||||
<FormField
|
||||
control={form.control}
|
||||
name="priority"
|
||||
label={labelWithHint(
|
||||
"Priority",
|
||||
"Lower numbers run first. Attachments with a priority run before attachments without one.",
|
||||
)}
|
||||
description="Optional. Leave blank to keep the default order: global, then teams, keys, tags, models."
|
||||
>
|
||||
{({ ref, value, onChange, ...field }) => (
|
||||
<Input
|
||||
{...field}
|
||||
ref={ref}
|
||||
type="number"
|
||||
step={1}
|
||||
value={value ?? ""}
|
||||
placeholder="e.g. 10"
|
||||
onChange={(event) => onChange(event.target.value === "" ? null : event.target.valueAsNumber)}
|
||||
/>
|
||||
)}
|
||||
</FormField>
|
||||
</FieldGroup>
|
||||
|
||||
{impactResult && <ImpactPreviewAlert impactResult={impactResult} />}
|
||||
|
|
|
|||
|
|
@ -79,4 +79,18 @@ describe("buildAttachmentData", () => {
|
|||
expect(result.tags).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe("priority", () => {
|
||||
it.each(["global", "specific"] as const)("should include priority for a %s scope", (scopeType) => {
|
||||
expect(buildAttachmentData({ policy_name: "p", priority: 0 }, scopeType).priority).toBe(0);
|
||||
});
|
||||
|
||||
it("should include a negative priority", () => {
|
||||
expect(buildAttachmentData({ policy_name: "p", priority: -5 }, "specific").priority).toBe(-5);
|
||||
});
|
||||
|
||||
it.each([undefined, null])("should omit priority when it is %s", (priority) => {
|
||||
expect(buildAttachmentData({ policy_name: "p", priority }, "specific")).not.toHaveProperty("priority");
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,13 +1,16 @@
|
|||
import { PolicyAttachmentCreateRequest } from "@/components/policies/types";
|
||||
|
||||
/**
|
||||
* Builds a PolicyAttachmentCreateRequest from form values.
|
||||
*
|
||||
* @param formValues - The raw form field values (from form.getFieldsValue)
|
||||
* @param scopeType - Whether the attachment is "global" or "specific"
|
||||
*/
|
||||
export interface AttachmentFormInput {
|
||||
policy_name: string;
|
||||
teams?: string[];
|
||||
keys?: string[];
|
||||
models?: string[];
|
||||
tags?: string[];
|
||||
priority?: number | null;
|
||||
}
|
||||
|
||||
export function buildAttachmentData(
|
||||
formValues: Record<string, any>,
|
||||
formValues: AttachmentFormInput,
|
||||
scopeType: "global" | "specific",
|
||||
): PolicyAttachmentCreateRequest {
|
||||
const data: PolicyAttachmentCreateRequest = {
|
||||
|
|
@ -21,5 +24,6 @@ export function buildAttachmentData(
|
|||
if (formValues.models && formValues.models.length > 0) data.models = formValues.models;
|
||||
if (formValues.tags && formValues.tags.length > 0) data.tags = formValues.tags;
|
||||
}
|
||||
if (typeof formValues.priority === "number") data.priority = formValues.priority;
|
||||
return data;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -44,6 +44,7 @@ export interface PolicyAttachment {
|
|||
keys: string[];
|
||||
models: string[];
|
||||
tags: string[];
|
||||
priority?: number | null;
|
||||
created_at?: string;
|
||||
updated_at?: string;
|
||||
created_by?: string;
|
||||
|
|
@ -78,6 +79,7 @@ export interface PolicyAttachmentCreateRequest {
|
|||
keys?: string[];
|
||||
models?: string[];
|
||||
tags?: string[];
|
||||
priority?: number;
|
||||
}
|
||||
|
||||
export interface PolicyListResponse {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue