Skip to content

Improve custom policy usage reconciliation#2836

Open
lahiruudayakumara wants to merge 2 commits into
wso2:mainfrom
lahiruudayakumara:issue-2802-custom-policy-usage-improvements
Open

Improve custom policy usage reconciliation#2836
lahiruudayakumara wants to merge 2 commits into
wso2:mainfrom
lahiruudayakumara:issue-2802-custom-policy-usage-improvements

Conversation

@lahiruudayakumara

Copy link
Copy Markdown
Contributor

Purpose

Ensure custom policy usages are consistently maintained for LLM providers and reduce database transaction overhead when storing multiple policy usages.

Resolves #2802

Goals

  • Reconcile custom policy usages for every LLM provider create and update operation.
  • Store multiple policy usages using a single database insert.
  • Preserve policy UUID deduplication and transactional rollback behavior.

Approach

  • Removed the conditional controlling custom policy usage reconciliation from the shared LLM provider create and update paths.
  • Updated replaceCustomPolicyUsagesTx to build one parameterized multi-row INSERT instead of executing one insert per policy UUID.
  • Retained the existing deletion-first replacement behavior within the transaction.
  • Added regression coverage for deduplication, rollback, and unconditional reconciliation.

User stories

  • As a platform administrator, I want LLM provider custom policy usages to remain accurate so that policies currently in use cannot be incorrectly deleted.
  • As a platform operator, I want policy usage persistence to minimize database round-trips.

Documentation

N/A — this is an internal persistence and performance improvement with no user-facing API or configuration changes.

Automation tests

  • Unit tests
    • Added coverage for deduplicated multi-policy usage insertion.
    • Added coverage for unconditional reconciliation through the standard update path.
    • Verified transaction rollback when policy usage insertion fails.
    • Full internal/repository test suite passes.
  • Integration tests
    • N/A — the affected repository behavior is covered using the repository database test suite.

Security checks

Samples

N/A — no sample changes are required.

Related PRs

N/A

Test environment

  • macOS
  • Go repository test suite
  • SQLite test database
  • JDK: N/A
  • Browser: N/A

@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@lahiruudayakumara, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 29 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 3ce84817-9ad6-41dc-94e2-2ec41ac787f7

📥 Commits

Reviewing files that changed from the base of the PR and between 929bf6b and 4448bd7.

📒 Files selected for processing (2)
  • platform-api/internal/repository/custom_policy.go
  • platform-api/internal/repository/llm_test.go
📝 Walkthrough

Walkthrough

LLM provider creation and updates now always reconcile custom policy usages. Usage UUIDs are deduplicated and inserted with one batched SQL statement. Tests cover duplicate inputs, rollback behavior, and clearing usages during plain updates.

Changes

LLM custom policy usage reconciliation

Layer / File(s) Summary
Batch usage insertion
platform-api/internal/repository/custom_policy.go
Deduplicates policy UUIDs and persists usage rows through a single multi-row INSERT.
Provider create and update reconciliation
platform-api/internal/repository/llm.go
Removes the reconciliation flag and always replaces custom policy usages during provider creation and updates.
Reconciliation test coverage
platform-api/internal/repository/llm_test.go
Verifies deduplication, rollback on insert failure, and clearing usages on plain update.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

Suggested reviewers: malinthaprasan, renuka-fernando, ashera96

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately reflects the main change: custom policy usage reconciliation.
Description check ✅ Passed The description includes all required sections and provides purpose, approach, tests, security, and environment details.
Linked Issues check ✅ Passed The PR satisfies #2802 by making reconciliation unconditional and batching deduplicated policy inserts.
Out of Scope Changes check ✅ Passed The changes stay within the linked issue scope and add tests for the same reconciliation behavior.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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.

Inline comments:
In `@platform-api/internal/repository/custom_policy.go`:
- Around line 244-261: Update the custom-policy usage INSERT flow around the
valuePlaceholders and args construction to avoid unbounded bind parameters.
Split unique policy UUIDs into bounded INSERT chunks within the existing
transaction, keeping each chunk below the lowest supported database parameter
limit and preserving deduplication and error handling.

In `@platform-api/internal/repository/llm_test.go`:
- Around line 102-104: Strengthen the assertion in the usage test around the
policy usage results to verify both distinct UUIDs, not just len(usages).
Compare the returned UUIDs order-independently against policy.UUID and
secondPolicy.UUID, while preserving the required deduplication expectation.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: eebe21d7-d477-40ee-87b1-ff7dd3b23df0

📥 Commits

Reviewing files that changed from the base of the PR and between 263fc3e and 929bf6b.

📒 Files selected for processing (3)
  • platform-api/internal/repository/custom_policy.go
  • platform-api/internal/repository/llm.go
  • platform-api/internal/repository/llm_test.go

Comment thread platform-api/internal/repository/custom_policy.go Outdated
Comment thread platform-api/internal/repository/llm_test.go
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Task]: Improvements for custom policy usage related logic

1 participant