Ensure llm-cost-based-ratelimit precedes llm-cost in generated YAML

[Saadha123]

Purpose

Ensures correct policy execution ordering in generated deployment YAML for LLM providers and proxies. When both llm-cost and llm-cost-based-ratelimit policies are present, llm-cost must execute after llm-cost-based-ratelimit — since cost tracking depends on rate limiting being evaluated first.

Goals

Guarantee that llm-cost-based-ratelimit always appears before llm-cost in the generated deployment YAML, regardless of the order in which they were stored or submitted.

Approach

Added an orderLLMPolicies helper in llm_deployment.go that scans the assembled policy slice and swaps llm-cost and llm-cost-based-ratelimit if they are out of order. The helper is called in both generateLLMProviderDeploymentYAML and generateLLMProxyDeploymentYAML after all policies are assembled, before marshalling to YAML. All other policies retain their relative positions.

User stories

As an API developer, when I attach both llm-cost and llm-cost-based-ratelimit policies to an LLM provider or proxy, I want the gateway to enforce rate limiting before cost tracking so that cost is only recorded for requests that pass the rate limit.

Documentation

N/A — internal policy ordering enforced at the platform layer; no user-facing behaviour change.

Automation tests

• Unit tests

  To be added — orderLLMPolicies should be unit tested covering: both policies in wrong order, both in correct order, only one present, neither present.

• Integration tests

  Existing llm-cost-based-ratelimit.feature integration tests cover the end-to-end deployment flow. No new cases required for this fix specifically.

Security checks

• Followed secure coding standards in http://wso2.com/technical-reports/wso2-secure-engineering-guidelines? yes
• Ran FindSecurityBugs plugin and verified report? yes
• Confirmed that this PR doesn't commit any keys, passwords, tokens, usernames, or other secrets? yes

Samples

N/A

Related PRs

N/A

Summary by CodeRabbit

• New Features

  • Added cost tracking and a cost-based rate limiting policy for finer LLM usage control.

• Improvements

  • Deployment now enforces policy ordering so cost-based limits are applied before cost accounting.
  • Streamlined policy version handling for more consistent behavior across deployments.

[coderabbitai]

Walkthrough

GenerateLLM YAML now adds two new policy-name constants, emits empty Version values for several generated policies, and reorders assembled policies so llm-cost-based-ratelimit precedes llm-cost before embedding into deployment YAML.

Changes

Cohort / File(s)	Summary
LLM deployment generation platform-api/src/internal/service/llm_deployment.go	Added policy name constants llm-cost and llm-cost-based-ratelimit. Updated generated policy calls to pass empty Version for api-key-auth, token-based-ratelimit, and advanced-ratelimit. Added and invoked orderLLMPolicies(...) to place llm-cost-based-ratelimit before llm-cost while preserving other relative ordering.
Tests updated platform-api/src/internal/service/llm_test.go	Adjusted test assertions and findPolicy(...) lookups to expect empty Version ("") instead of "v0" for api-key-auth, token-based-ratelimit, and advanced-ratelimit policies.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I hopped through YAML, quiet and spry,
Two policies met and I nudged one by.
Cost now takes a gentle step behind rate,
I twitched my nose — all set, orderly and great. 🥕✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: ensuring llm-cost-based-ratelimit precedes llm-cost in generated YAML, which is the primary objective and implementation focus of this PR.
Description check	✅ Passed	The description covers all required sections: Purpose clearly explains the policy ordering issue, Goals states the guarantee needed, Approach describes the implementation, User stories provide context, and Security/Documentation sections are appropriately addressed.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

📝 Coding Plan

• Generate coding plan for human review comments

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

platform-api/src/internal/service/llm_deployment.go (1)

1440-1447: Edge case: multiple policies with the same name.

The loop records only the last index for each policy name. If duplicate llm-cost policies exist (e.g., added via direct append() calls at lines 620, 646, 684, 711), only the last occurrence is considered for swapping.

Example scenario:

• Input: [llm-cost, llm-cost-based-ratelimit, llm-cost]
• costIdx = 2, rateLimitIdx = 1
• Condition costIdx < rateLimitIdx is false → no swap
• Result: first llm-cost remains at index 0, before llm-cost-based-ratelimit

This is likely rare since addOrAppendPolicyPath prevents duplicates, but consider tracking the first occurrence of llm-cost instead:

♻️ Optional: track first llm-cost occurrence

 func orderLLMPolicies(policies []api.LLMPolicy) []api.LLMPolicy {
 	costIdx := -1
 	rateLimitIdx := -1
 	for i, p := range policies {
 		switch p.Name {
 		case llmCostPolicyName:
-			costIdx = i
+			if costIdx == -1 {
+				costIdx = i
+			}
 		case llmCostBasedRateLimitPolicyName:
 			rateLimitIdx = i
 		}
 	}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@platform-api/src/internal/service/llm_deployment.go` around lines 1440 -
1447, The loop over policies only records the last index for llm-cost and
llm-cost-based-ratelimit, causing duplicate llm-cost entries to be misordered;
update the scan that sets costIdx and rateLimitIdx in the block iterating over
policies so it records the first occurrence (e.g., initialize
costIdx/rateLimitIdx to -1 and only set costIdx = i or rateLimitIdx = i when the
current value is still -1) for names llmCostPolicyName and
llmCostBasedRateLimitPolicyName; this preserves the intended swap logic and
complements existing deduplication in addOrAppendPolicyPath.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@platform-api/src/internal/service/llm_deployment.go`:
- Around line 1440-1447: The loop over policies only records the last index for
llm-cost and llm-cost-based-ratelimit, causing duplicate llm-cost entries to be
misordered; update the scan that sets costIdx and rateLimitIdx in the block
iterating over policies so it records the first occurrence (e.g., initialize
costIdx/rateLimitIdx to -1 and only set costIdx = i or rateLimitIdx = i when the
current value is still -1) for names llmCostPolicyName and
llmCostBasedRateLimitPolicyName; this preserves the intended swap logic and
complements existing deduplication in addOrAppendPolicyPath.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 54cd0631-e741-40b8-a9ac-4f68671e9f2e

📥 Commits

Reviewing files that changed from the base of the PR and between be98e0c and e3debe7.

📒 Files selected for processing (1)

• platform-api/src/internal/service/llm_deployment.go

[coderabbitai]

🧹 Nitpick comments (1)

platform-api/src/internal/service/llm_test.go (1)

432-432: Add explicit regression tests for llm-cost-based-ratelimit ordering.

Line 432 and related assertions verify policy lookup/version, but this PR’s core guarantee is relative order (llm-cost-based-ratelimit before llm-cost). Please add direct tests for orderLLMPolicies (or end-to-end provider/proxy policy slices) covering wrong order, correct order, only one present, and neither present.

🧪 Suggested test addition

+func TestOrderLLMPolicies(t *testing.T) {
+	tests := []struct {
+		name     string
+		input    []api.LLMPolicy
+		expected []string
+	}{
+		{
+			name: "swaps when llm-cost appears before llm-cost-based-ratelimit",
+			input: []api.LLMPolicy{
+				{Name: "foo", Version: ""},
+				{Name: "llm-cost", Version: ""},
+				{Name: "llm-cost-based-ratelimit", Version: ""},
+				{Name: "bar", Version: ""},
+			},
+			expected: []string{"foo", "llm-cost-based-ratelimit", "llm-cost", "bar"},
+		},
+		{
+			name: "keeps order when already correct",
+			input: []api.LLMPolicy{
+				{Name: "llm-cost-based-ratelimit", Version: ""},
+				{Name: "llm-cost", Version: ""},
+			},
+			expected: []string{"llm-cost-based-ratelimit", "llm-cost"},
+		},
+		{
+			name: "no change when only one exists",
+			input: []api.LLMPolicy{
+				{Name: "llm-cost", Version: ""},
+			},
+			expected: []string{"llm-cost"},
+		},
+		{
+			name: "no change when neither exists",
+			input: []api.LLMPolicy{
+				{Name: "advanced-ratelimit", Version: ""},
+			},
+			expected: []string{"advanced-ratelimit"},
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			got := orderLLMPolicies(tc.input)
+			if len(got) != len(tc.expected) {
+				t.Fatalf("expected %d policies, got %d", len(tc.expected), len(got))
+			}
+			for i, name := range tc.expected {
+				if got[i].Name != name {
+					t.Fatalf("expected policy[%d]=%s, got=%s", i, name, got[i].Name)
+				}
+			}
+		})
+	}
+}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@platform-api/src/internal/service/llm_test.go` at line 432, Add explicit
regression tests that verify the relative ordering guarantee that
orderLLMPolicies enforces: write unit tests that call orderLLMPolicies (or build
the provider/proxy policy slices and assert their final order) for four
scenarios—input where "llm-cost" appears before "llm-cost-based-ratelimit"
(wrong order) and assert the output reorders them, input already in the correct
order and assert it remains unchanged, input containing only "llm-cost" and
assert the single policy is preserved, and input containing neither policy and
assert the slice is unchanged; use the existing helper findPolicy to assert
presence/positions and reference the orderLLMPolicies function (and the policy
names "llm-cost-based-ratelimit" and "llm-cost") when locating the code to
modify.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@platform-api/src/internal/service/llm_test.go`:
- Line 432: Add explicit regression tests that verify the relative ordering
guarantee that orderLLMPolicies enforces: write unit tests that call
orderLLMPolicies (or build the provider/proxy policy slices and assert their
final order) for four scenarios—input where "llm-cost" appears before
"llm-cost-based-ratelimit" (wrong order) and assert the output reorders them,
input already in the correct order and assert it remains unchanged, input
containing only "llm-cost" and assert the single policy is preserved, and input
containing neither policy and assert the slice is unchanged; use the existing
helper findPolicy to assert presence/positions and reference the
orderLLMPolicies function (and the policy names "llm-cost-based-ratelimit" and
"llm-cost") when locating the code to modify.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e1632609-456f-472c-822f-75807d5677f8

📥 Commits

Reviewing files that changed from the base of the PR and between c103647 and 182faca.

📒 Files selected for processing (1)

• platform-api/src/internal/service/llm_test.go