Update template ID/handle to groupId and update REST APIs#2316
Merged
Conversation
Tharanidk
requested review from
AnuGayan,
Arshardh,
CrowleyRajapakse,
HeshanSudarshana,
HiranyaKavishani,
Krishanx92,
PasanT9,
RakhithaRR,
Tharsanan1,
VirajSalaka,
ashera96,
chamilaadhi,
dushaniw,
hisanhunais,
malinthaprasan,
pubudu538,
renuka-fernando,
senthuran16,
tgtshanika,
tharikaGitHub and
tharindu1st
as code owners
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis change reworks LLM provider template versioning to be scoped by a stable Sequence Diagram(s)sequenceDiagram
participant LLMHandler
participant LLMProviderTemplateService
participant LLMProviderTemplateRepo
LLMHandler->>LLMProviderTemplateService: CreateVersion(orgUUID, groupID, req)
LLMProviderTemplateService->>LLMProviderTemplateRepo: CountVersions(groupID, orgUUID)
LLMProviderTemplateRepo-->>LLMProviderTemplateService: version count
LLMProviderTemplateService->>LLMProviderTemplateRepo: Create(new version, groupID)
LLMHandler->>LLMProviderTemplateService: SetEnabledByHandle(orgUUID, handle, enabled)
LLMProviderTemplateService->>LLMProviderTemplateRepo: GetByVersion(groupID, orgUUID, version)
LLMProviderTemplateService->>LLMProviderTemplateRepo: CountProvidersUsingTemplate(groupID, orgUUID, version)
LLMProviderTemplateService->>LLMProviderTemplateRepo: SetEnabled(groupID, orgUUID, version, enabled)
LLMProviderTemplateRepo-->>LLMHandler: updated template version
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
platform-api/src/internal/repository/llm.go (1)
551-581: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRecompute
is_latestwithin the same managed-by bucket.
ListandCountnow group latest rows by(group_id, wso2/non-wso2 bucket), but this recompute checks all rows in the group. Use the deleted row’s bucket in the remaining/latest count and replacement update.🤖 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 `@platform-api/src/internal/repository/llm.go` around lines 551 - 581, The recompute logic in the delete flow for llm_provider_templates is still scoped only by group_id and organization_uuid, but latest rows are now grouped by the managed-by bucket used by List/Count. Update the delete path in the repository method that performs this cleanup so the remaining/latest count and the fallback UPDATE both include the deleted row’s bucket in their WHERE/filter conditions. Reuse the same bucket discriminator used elsewhere in the llm repository, and ensure the replacement latest row is chosen only from that bucket within the same group/org.
🧹 Nitpick comments (1)
portals/ai-workspace/src/apis/providerTemplateApis.ts (1)
134-134: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRename the exported parameter to
groupId.The JSDoc is now correct, but the public function signatures still expose
templateId. Keeping the old name on version-scoped helpers makes the new contract easier to misuse at call sites. Renaming the parameter here would keep the wrapper API aligned with the OpenAPI change.Also applies to: 159-159, 189-189, 234-234, 264-264
🤖 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 `@portals/ai-workspace/src/apis/providerTemplateApis.ts` at line 134, Rename the exported version-route helper parameter from templateId to groupId in the providerTemplateApis functions so the public API matches the updated JSDoc and OpenAPI contract. Update the affected wrapper signatures and any internal references in the related helper methods (the same rename also applies to the other listed helper definitions) so call sites clearly pass the template family group id instead of the old per-version handle name.
🤖 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/src/internal/repository/llm.go`:
- Around line 359-367: The anti-join query in the llm repository is referencing
an outer table alias that does not exist, so the SQL will fail. Update the query
in the repository method that builds the llm_provider_templates lookup to alias
the outer table as t, and make sure all outer-column references in the NOT
EXISTS clause consistently use that alias so the subquery correlates correctly.
In `@platform-api/src/internal/service/llm.go`:
- Line 475: The provider-usage lookup is still treating the first argument as a
template handle, but the call sites in llm.go now pass a group ID. Update the
repository logic behind CountProvidersUsingTemplate (or introduce a new
groupId-specific usage count method) so it filters by group_id directly, and
make sure the callers in the affected LLM service paths use the group-aware
method consistently.
In `@platform-api/src/resources/openapi.yaml`:
- Around line 9127-9139: The schema text in openapi.yaml still has version-route
examples using {id} even though this section now defines {groupId} as the
/versions route parameter. Update the remaining `version` and `enabled` property
descriptions in this schema block to reference
`/llm-provider-templates/{groupId}/versions...` so the route examples are
consistent with the `groupId`-based version routes.
In
`@portals/ai-workspace/src/pages/appShell/appShellPages/providerTemplate/ProviderTemplateOverview.tsx`:
- Around line 210-216: The version-loading logic in ProviderTemplateOverview is
using only template.groupId, which leaves templates without a groupId unable to
load versions and can keep stale data visible. Update the version fetch path to
use the same fallback as the rest of the component, matching the
template.groupId ?? template.id pattern used for version operations, and clear
versions before starting the request so the menu refreshes correctly. Apply the
same fallback consistently anywhere getProviderTemplateVersions is called in
this component.
---
Outside diff comments:
In `@platform-api/src/internal/repository/llm.go`:
- Around line 551-581: The recompute logic in the delete flow for
llm_provider_templates is still scoped only by group_id and organization_uuid,
but latest rows are now grouped by the managed-by bucket used by List/Count.
Update the delete path in the repository method that performs this cleanup so
the remaining/latest count and the fallback UPDATE both include the deleted
row’s bucket in their WHERE/filter conditions. Reuse the same bucket
discriminator used elsewhere in the llm repository, and ensure the replacement
latest row is chosen only from that bucket within the same group/org.
---
Nitpick comments:
In `@portals/ai-workspace/src/apis/providerTemplateApis.ts`:
- Line 134: Rename the exported version-route helper parameter from templateId
to groupId in the providerTemplateApis functions so the public API matches the
updated JSDoc and OpenAPI contract. Update the affected wrapper signatures and
any internal references in the related helper methods (the same rename also
applies to the other listed helper definitions) so call sites clearly pass the
template family group id instead of the old per-version handle name.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ac3092a1-7ec1-4bc1-9334-d5dfea08c3f9
📒 Files selected for processing (10)
platform-api/src/internal/repository/llm.goplatform-api/src/internal/service/llm.goplatform-api/src/internal/service/llm_provider_template_test.goplatform-api/src/resources/openapi.yamlportals/ai-workspace/src/apis/providerTemplateApis.tsportals/ai-workspace/src/contexts/llmProvider/providerTemplate/ProviderTemplateContext.tsxportals/ai-workspace/src/pages/appShell/appShellPages/providerTemplate/CreateProviderTemplateVersion.tsxportals/ai-workspace/src/pages/appShell/appShellPages/providerTemplate/ProviderTemplateOverview.tsxportals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/AddNewProvider/TemplateVersionDialog.tsxportals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/ServiceProviderNew.tsx
Contributor
Author
|
@CodeRabbit please review |
Contributor
|
✅ Action performedReview finished.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
portals/ai-workspace/src/contexts/llmProvider/providerTemplate/ProviderTemplateContext.tsx (1)
52-54: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRequire
groupIdfor version-specific fetches.Line 77 falls back to
handlewhenversionis present butgroupIdis missing. Since the version endpoint is now group-scoped, this can fetch the wrong path whenhandleandgroupIddiffer.Proposed fix
const fetchTemplate = useCallback(async () => { if (!handle || !organizationId) { setTemplate(null); setIsLoading(false); return; } + + if (version && !groupId) { + setTemplate(null); + setError(new Error('Template group ID is missing')); + setIsLoading(false); + return; + } try { setIsLoading(true); setError(null); - const fetchedTemplate = version - ? await providerTemplateApis.getProviderTemplateVersion(groupId ?? handle, version, organizationId, PLATFORM_API_BASE_URL) - : await providerTemplateApis.getProviderTemplate(handle, organizationId, PLATFORM_API_BASE_URL); + const fetchedTemplate = version + ? await providerTemplateApis.getProviderTemplateVersion(groupId, version, organizationId, PLATFORM_API_BASE_URL) + : await providerTemplateApis.getProviderTemplate(handle, organizationId, PLATFORM_API_BASE_URL); setTemplate(fetchedTemplate);Also applies to: 67-78
🤖 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 `@portals/ai-workspace/src/contexts/llmProvider/providerTemplate/ProviderTemplateContext.tsx` around lines 52 - 54, The version-specific fetch path in ProviderTemplateContext should not fall back to handle when version is set, because the endpoint is group-scoped and can resolve the wrong resource. Update the logic in the provider template fetch flow to require groupId whenever version is present, and use groupId consistently for the versioned request path instead of handle. If needed, add validation in the context/types around ProviderTemplateContext so callers cannot request a versioned template without a groupId.platform-api/src/resources/openapi.yaml (1)
2396-2463: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winKeep the versions path parameter name aligned with the handler.
These routes now document
{groupId}, but the supplied handler implementation for the same operations still readsr.PathValue("id")inplatform-api/src/internal/handler/llm.go:185-260. Please align the spec and handler/route binding together; otherwise the public contract and implementation will drift.🤖 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 `@platform-api/src/resources/openapi.yaml` around lines 2396 - 2463, The versions endpoints now document the path parameter as groupId, but the handler still reads a different path value, so align the contract and implementation. Update the LLM provider template version routes in openapi.yaml and the corresponding handler methods in llm.go (the list/create version flow) so they all use the same parameter name, and make sure the route binding and r.PathValue lookup match the shared symbol for the template family identifier.
🧹 Nitpick comments (1)
platform-api/src/internal/repository/llm.go (1)
354-385: 🚀 Performance & Scalability | 🔵 TrivialConsider index support for the per-group latest-row anti-join.
The correlated
NOT EXISTSselects the newest row per(organization_uuid, group_id, managed_by bucket). On larger datasets this benefits from a composite index covering(organization_uuid, group_id, managed_by, created_at, uuid)so the subquery avoids repeated scans.🤖 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 `@platform-api/src/internal/repository/llm.go` around lines 354 - 385, The LLMProviderTemplateRepo.List query uses a correlated NOT EXISTS anti-join to pick the latest row per organization/group/managed_by bucket, which can become slow on large tables. Add or update the database indexing strategy used by List so the subquery can use a composite index on LLM provider templates covering organization_uuid, group_id, managed_by, created_at, and uuid; keep the query logic in List and the scanTemplate path unchanged.
🤖 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/src/internal/service/llm_provider_template_test.go`:
- Around line 221-223: The test is incorrectly using repo.created as a proxy for
whether Create was invoked, but that field only reflects stubbed return data and
does not guarantee call coverage. Update the mock used by
llm_provider_template_test.go to explicitly track Create calls (for example with
a createCalled flag on the repository stub) and then assert that this flag
remains false in this case, so the check accurately verifies that Create was not
called.
---
Outside diff comments:
In `@platform-api/src/resources/openapi.yaml`:
- Around line 2396-2463: The versions endpoints now document the path parameter
as groupId, but the handler still reads a different path value, so align the
contract and implementation. Update the LLM provider template version routes in
openapi.yaml and the corresponding handler methods in llm.go (the list/create
version flow) so they all use the same parameter name, and make sure the route
binding and r.PathValue lookup match the shared symbol for the template family
identifier.
In
`@portals/ai-workspace/src/contexts/llmProvider/providerTemplate/ProviderTemplateContext.tsx`:
- Around line 52-54: The version-specific fetch path in ProviderTemplateContext
should not fall back to handle when version is set, because the endpoint is
group-scoped and can resolve the wrong resource. Update the logic in the
provider template fetch flow to require groupId whenever version is present, and
use groupId consistently for the versioned request path instead of handle. If
needed, add validation in the context/types around ProviderTemplateContext so
callers cannot request a versioned template without a groupId.
---
Nitpick comments:
In `@platform-api/src/internal/repository/llm.go`:
- Around line 354-385: The LLMProviderTemplateRepo.List query uses a correlated
NOT EXISTS anti-join to pick the latest row per organization/group/managed_by
bucket, which can become slow on large tables. Add or update the database
indexing strategy used by List so the subquery can use a composite index on LLM
provider templates covering organization_uuid, group_id, managed_by, created_at,
and uuid; keep the query logic in List and the scanTemplate path unchanged.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 149f9c6a-4408-470b-981f-e53793b93d86
📒 Files selected for processing (15)
platform-api/src/internal/constants/error.goplatform-api/src/internal/handler/llm.goplatform-api/src/internal/repository/interfaces.goplatform-api/src/internal/repository/llm.goplatform-api/src/internal/service/artifact_import_lifecycle_test.goplatform-api/src/internal/service/llm.goplatform-api/src/internal/service/llm_provider_template_test.goplatform-api/src/resources/openapi.yamlportals/ai-workspace/cypress/e2e/001-providers/002-provider-secret-management.cy.jsportals/ai-workspace/cypress/e2e/002-mcp-proxies/003-mcp-secret-management.cy.jsportals/ai-workspace/src/apis/providerTemplateApis.tsportals/ai-workspace/src/contexts/llmProvider/providerTemplate/ProviderTemplateContext.tsxportals/ai-workspace/src/contexts/llmProvider/providerTemplate/ProviderTemplatesContext.tsxportals/ai-workspace/src/pages/appShell/appShellPages/providerTemplate/ProviderTemplateOverview.tsxportals/ai-workspace/src/pages/appShell/appShellPages/providerTemplate/ProviderTemplatesList.tsx
💤 Files with no reviewable changes (3)
- platform-api/src/internal/service/artifact_import_lifecycle_test.go
- platform-api/src/internal/repository/interfaces.go
- portals/ai-workspace/src/apis/providerTemplateApis.ts
✅ Files skipped from review due to trivial changes (1)
- portals/ai-workspace/src/pages/appShell/appShellPages/providerTemplate/ProviderTemplatesList.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- portals/ai-workspace/src/pages/appShell/appShellPages/providerTemplate/ProviderTemplateOverview.tsx
- platform-api/src/internal/service/llm.go
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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
`@portals/ai-workspace/cypress/e2e/005-provider-templates/005-custom-provider-template.cy.js`:
- Around line 253-261: The cleanup helper deleteTemplateVersion currently treats
409 as an acceptable delete result, but that means a version still in use can be
left behind and pollute later tests. Update the expectation in
deleteTemplateVersion so 409 is not considered success; instead, make afterEach
retry deletion after the provider is removed or fail cleanup when a 409 is
returned, while keeping the existing handling in requestWithAuth and the
template version delete flow.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 64f7913f-e072-4f31-b44d-1d0441ab96bb
📒 Files selected for processing (2)
platform-api/src/internal/service/llm_provider_template_test.goportals/ai-workspace/cypress/e2e/005-provider-templates/005-custom-provider-template.cy.js
🚧 Files skipped from review as they are similar to previous changes (1)
- platform-api/src/internal/service/llm_provider_template_test.go
Tharanidk
force-pushed
the
main-platform-portal
branch
2 times, most recently
from
b3918f2 to
ab7d3b0
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/src/internal/service/llm.go`:
- Around line 134-136: The validation in llm.go currently only checks that
req.Metadata.EndpointUrl is non-empty, so update the endpoint handling in the
relevant request flow to parse the trimmed value and reject anything that is not
an absolute HTTP or HTTPS URL before continuing. Use the existing validation
block around req.Metadata, utils.ValueOrEmpty, and constants.ErrInvalidInput,
and keep the check close to where the template is persisted so only usable
endpoint URLs are accepted.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1eada63f-8753-453b-a0e9-12da4693c7a6
📒 Files selected for processing (19)
platform-api/src/internal/constants/error.goplatform-api/src/internal/handler/llm.goplatform-api/src/internal/repository/interfaces.goplatform-api/src/internal/repository/llm.goplatform-api/src/internal/service/artifact_import_lifecycle_test.goplatform-api/src/internal/service/llm.goplatform-api/src/internal/service/llm_provider_template_test.goplatform-api/src/resources/openapi.yamlportals/ai-workspace/cypress/e2e/001-providers/002-provider-secret-management.cy.jsportals/ai-workspace/cypress/e2e/002-mcp-proxies/003-mcp-secret-management.cy.jsportals/ai-workspace/cypress/e2e/005-provider-templates/005-custom-provider-template.cy.jsportals/ai-workspace/src/apis/providerTemplateApis.tsportals/ai-workspace/src/contexts/llmProvider/providerTemplate/ProviderTemplateContext.tsxportals/ai-workspace/src/contexts/llmProvider/providerTemplate/ProviderTemplatesContext.tsxportals/ai-workspace/src/pages/appShell/appShellPages/providerTemplate/CreateProviderTemplateVersion.tsxportals/ai-workspace/src/pages/appShell/appShellPages/providerTemplate/ProviderTemplateOverview.tsxportals/ai-workspace/src/pages/appShell/appShellPages/providerTemplate/ProviderTemplatesList.tsxportals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/AddNewProvider/TemplateVersionDialog.tsxportals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/ServiceProviderNew.tsx
💤 Files with no reviewable changes (2)
- platform-api/src/internal/repository/interfaces.go
- platform-api/src/internal/service/artifact_import_lifecycle_test.go
✅ Files skipped from review due to trivial changes (1)
- platform-api/src/internal/constants/error.go
🚧 Files skipped from review as they are similar to previous changes (15)
- portals/ai-workspace/src/pages/appShell/appShellPages/providerTemplate/CreateProviderTemplateVersion.tsx
- portals/ai-workspace/src/pages/appShell/appShellPages/providerTemplate/ProviderTemplatesList.tsx
- portals/ai-workspace/cypress/e2e/002-mcp-proxies/003-mcp-secret-management.cy.js
- portals/ai-workspace/cypress/e2e/001-providers/002-provider-secret-management.cy.js
- portals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/AddNewProvider/TemplateVersionDialog.tsx
- portals/ai-workspace/cypress/e2e/005-provider-templates/005-custom-provider-template.cy.js
- portals/ai-workspace/src/contexts/llmProvider/providerTemplate/ProviderTemplateContext.tsx
- portals/ai-workspace/src/pages/appShell/appShellPages/providerTemplate/ProviderTemplateOverview.tsx
- portals/ai-workspace/src/apis/providerTemplateApis.ts
- portals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/ServiceProviderNew.tsx
- platform-api/src/internal/handler/llm.go
- portals/ai-workspace/src/contexts/llmProvider/providerTemplate/ProviderTemplatesContext.tsx
- platform-api/src/internal/service/llm_provider_template_test.go
- platform-api/src/internal/repository/llm.go
- platform-api/src/resources/openapi.yaml
Tharanidk
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
portals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/ServiceProviderNew.tsx (1)
412-459: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winDiscard outdated template-selection requests.
handleSelectTemplatenow waits on one or two API calls before updatingpendingTemplateor the selected version state, but it never checks whether a newer click happened meanwhile. Rapidly picking a different template can reopen the dialog or apply the older selection after the user has already moved on. Gate the result with a selection token, or temporarily disable reselection while the lookup is in flight.🤖 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 `@portals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/ServiceProviderNew.tsx` around lines 412 - 459, `handleSelectTemplate` can still apply an older template after a newer click because the async lookup for `providerTemplateApis.getProviderTemplate` and `getProviderTemplateVersions` is not guarded. Add a selection token or in-flight guard inside `handleSelectTemplate` so only the latest invocation can call `applyTemplateSelection`, `setPendingTemplate`, or `setVersionDialogOpen`, and ignore stale responses if a newer selection starts before the current one finishes.portals/ai-workspace/src/contexts/llmProvider/providerTemplate/ProviderTemplateContext.tsx (1)
64-83: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winIgnore stale fetches when
handlechanges.
fetchTemplatecommits state as soon as the request resolves, but it does not guard against an older request finishing after a newer one. In flows likeServiceProviderNew, quickly switching templates can leave this context showing the wrong template. Track a request id or cancel obsolete calls before updating state.🤖 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 `@portals/ai-workspace/src/contexts/llmProvider/providerTemplate/ProviderTemplateContext.tsx` around lines 64 - 83, fetchTemplate in ProviderTemplateContext should ignore stale responses when handle or organizationId changes, because an older providerTemplateApis.getProviderTemplate call can finish after a newer one and overwrite the current template state. Add a request guard in fetchTemplate (for example a request id or cancellation flag scoped to the useCallback/useEffect flow) and only call setTemplate, setError, and setIsLoading for the latest in-flight request so ServiceProviderNew cannot show an outdated template.
🧹 Nitpick comments (2)
platform-api/src/resources/openapi.yaml (1)
2239-2240: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueDrop the redundant
organizationIdquery param from the portal template client. The OpenAPI contract scopes these template operations from the JWT already, so keepingorganizationIdinportals/ai-workspace/src/apis/providerTemplateApis.tsleaves the handwritten client out of sync with the documented request shape.🤖 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 `@platform-api/src/resources/openapi.yaml` around lines 2239 - 2240, Remove the redundant organizationId query parameter from the portal template client so it matches the OpenAPI contract that derives organization scope from the JWT. Update the request-building logic in providerTemplateApis.ts, especially the template-related API methods, to stop accepting or forwarding organizationId and keep the handwritten client aligned with the documented request shape.platform-api/src/internal/utils/llm_provider_template_loader_test.go (1)
81-95: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAlso assert built-in
groupIduniqueness.This only checks for empty values. Since
groupIdnow defines the version family key, two built-ins sharing the same value would still pass here while merging into one family at runtime. A small seen-set assertion would catch that class of metadata mistakes early.Proposed test update
func TestLoadLLMProviderTemplates_BuiltInsHaveGroupId(t *testing.T) { dir := filepath.Join("..", "..", "resources", "default-llm-provider-templates") got, err := LoadLLMProviderTemplatesFromDirectory(dir) if err != nil { t.Fatalf("load built-ins: %v", err) } if len(got) == 0 { t.Fatal("expected built-in templates to load") } + seen := map[string]string{} for _, tpl := range got { if tpl.GroupID == "" { t.Errorf("built-in %q has empty groupId", tpl.ID) } + if prev, ok := seen[tpl.GroupID]; ok { + t.Errorf("groupId %q is used by both %q and %q", tpl.GroupID, prev, tpl.ID) + continue + } + seen[tpl.GroupID] = tpl.ID } }🤖 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 `@platform-api/src/internal/utils/llm_provider_template_loader_test.go` around lines 81 - 95, Update TestLoadLLMProviderTemplates_BuiltInsHaveGroupId to also verify built-in GroupID values are unique, not just non-empty. Add a seen-set while iterating over the templates loaded by LoadLLMProviderTemplatesFromDirectory, and fail the test if any two built-ins share the same tpl.GroupID; keep the existing empty-value check and use tpl.ID in the error message for easy identification.
🤖 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/src/internal/handler/llm.go`:
- Around line 80-96: `parseTemplateQueryGroupID` currently conflates a missing
`groupId` with a present-but-empty one, which lets malformed version queries
fall back to the base-template flow. Update `parseTemplateQueryGroupID` in
`llm.go` to return presence separately from the parsed value (or return an
error), then adjust the POST/GET handler paths that use it so `query=groupId:`
is rejected with a 400 instead of continuing. Make sure the version-specific
flow in the `CreateVersion` and `ListVersions` handling only runs when `groupId`
is actually present and non-blank, and add a regression test in
`parse_template_query_test.go` for the empty `groupId` case.
In `@platform-api/src/resources/openapi.yaml`:
- Around line 2233-2240: The create-template operation currently documents two
different payload shapes but only exposes LLMProviderTemplate in the request
body schema. Update the OpenAPI definition for the create flow in openapi.yaml
so the requestBody matches both modes, using a oneOf (or a separate operation)
that includes CreateLLMProviderTemplateVersionRequest for the groupId query path
and LLMProviderTemplate for the new-family path. Ensure the operation’s schema
and description stay aligned so generated clients and validators see the correct
contract.
- Around line 2232-2258: The createLLMProviderTemplate API change removes the
existing version-create path compatibility while still on v0.9, so preserve the
retired route as a deprecated alias or equivalent operation instead of replacing
it outright. Update the OpenAPI definition to keep both the old
/llm-provider-templates/{groupId}/versions route and the new query-based POST
behavior, ensuring existing clients continue to work during this version.
---
Outside diff comments:
In
`@portals/ai-workspace/src/contexts/llmProvider/providerTemplate/ProviderTemplateContext.tsx`:
- Around line 64-83: fetchTemplate in ProviderTemplateContext should ignore
stale responses when handle or organizationId changes, because an older
providerTemplateApis.getProviderTemplate call can finish after a newer one and
overwrite the current template state. Add a request guard in fetchTemplate (for
example a request id or cancellation flag scoped to the useCallback/useEffect
flow) and only call setTemplate, setError, and setIsLoading for the latest
in-flight request so ServiceProviderNew cannot show an outdated template.
In
`@portals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/ServiceProviderNew.tsx`:
- Around line 412-459: `handleSelectTemplate` can still apply an older template
after a newer click because the async lookup for
`providerTemplateApis.getProviderTemplate` and `getProviderTemplateVersions` is
not guarded. Add a selection token or in-flight guard inside
`handleSelectTemplate` so only the latest invocation can call
`applyTemplateSelection`, `setPendingTemplate`, or `setVersionDialogOpen`, and
ignore stale responses if a newer selection starts before the current one
finishes.
---
Nitpick comments:
In `@platform-api/src/internal/utils/llm_provider_template_loader_test.go`:
- Around line 81-95: Update TestLoadLLMProviderTemplates_BuiltInsHaveGroupId to
also verify built-in GroupID values are unique, not just non-empty. Add a
seen-set while iterating over the templates loaded by
LoadLLMProviderTemplatesFromDirectory, and fail the test if any two built-ins
share the same tpl.GroupID; keep the existing empty-value check and use tpl.ID
in the error message for easy identification.
In `@platform-api/src/resources/openapi.yaml`:
- Around line 2239-2240: Remove the redundant organizationId query parameter
from the portal template client so it matches the OpenAPI contract that derives
organization scope from the JWT. Update the request-building logic in
providerTemplateApis.ts, especially the template-related API methods, to stop
accepting or forwarding organizationId and keep the handwritten client aligned
with the documented request shape.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0f6297d8-ff93-4ece-96e2-565c9913b778
📒 Files selected for processing (18)
platform-api/src/internal/handler/llm.goplatform-api/src/internal/handler/parse_template_query_test.goplatform-api/src/internal/service/llm.goplatform-api/src/internal/utils/llm_provider_template_loader.goplatform-api/src/internal/utils/llm_provider_template_loader_test.goplatform-api/src/resources/default-llm-provider-templates/anthropic-template.yamlplatform-api/src/resources/default-llm-provider-templates/awsbedrock-template.yamlplatform-api/src/resources/default-llm-provider-templates/azureaifoundry-template.yamlplatform-api/src/resources/default-llm-provider-templates/azureopenai-template.yamlplatform-api/src/resources/default-llm-provider-templates/gemini-template.yamlplatform-api/src/resources/default-llm-provider-templates/mistral-template.yamlplatform-api/src/resources/default-llm-provider-templates/openai-template.yamlplatform-api/src/resources/openapi.yamlportals/ai-workspace/cypress/e2e/005-provider-templates/005-custom-provider-template.cy.jsportals/ai-workspace/src/apis/providerTemplateApis.tsportals/ai-workspace/src/contexts/llmProvider/providerTemplate/ProviderTemplateContext.tsxportals/ai-workspace/src/pages/appShell/appShellPages/providerTemplate/ProviderTemplateOverview.tsxportals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/ServiceProviderNew.tsx
✅ Files skipped from review due to trivial changes (2)
- platform-api/src/resources/default-llm-provider-templates/gemini-template.yaml
- platform-api/src/resources/default-llm-provider-templates/awsbedrock-template.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- platform-api/src/internal/service/llm.go
Tharanidk
force-pushed
the
main-platform-portal
branch
from
786c3cd to
17b7955
Compare
Contributor
Author
|
@CodeRabbit please review |
Contributor
|
✅ Action performedReview finished.
|
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
platform-api/src/internal/handler/llm_template_integration_test.go (1)
69-72: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse parameterized query for INSERT instead of string concatenation.
The
tmplOrgvalue is a hardcoded test constant, so there's no real injection risk here, but the pattern is inconsistent with the parameterizeddb.Execcalls used elsewhere in this file (e.g., lines 355-358) and static analysis flags it. Switch to placeholders for consistency and to avoid setting a bad precedent for future edits to this constant.🔧 Proposed fix
- if _, err = db.Exec(`INSERT INTO organizations (uuid, handle, display_name, region, created_at, updated_at) - VALUES ('` + tmplOrg + `', 'test-org', 'Test Org', 'default', datetime('now'), datetime('now'))`); err != nil { + if _, err = db.Exec(`INSERT INTO organizations (uuid, handle, display_name, region, created_at, updated_at) + VALUES (?, 'test-org', 'Test Org', 'default', datetime('now'), datetime('now'))`, tmplOrg); err != nil {🤖 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 `@platform-api/src/internal/handler/llm_template_integration_test.go` around lines 69 - 72, The INSERT in the integration test uses string concatenation for tmplOrg instead of the parameterized db.Exec style used elsewhere. Update the organization insert in llm_template_integration_test.go to use placeholders and pass tmplOrg as an argument, keeping the test data flow consistent and matching the existing Exec patterns in this file.Source: Linters/SAST tools
🤖 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 `@platform-api/src/internal/handler/llm_template_integration_test.go`:
- Around line 69-72: The INSERT in the integration test uses string
concatenation for tmplOrg instead of the parameterized db.Exec style used
elsewhere. Update the organization insert in llm_template_integration_test.go to
use placeholders and pass tmplOrg as an argument, keeping the test data flow
consistent and matching the existing Exec patterns in this file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f74539d8-0973-4af8-91ff-ee9ecc312360
📒 Files selected for processing (28)
platform-api/src/internal/constants/error.goplatform-api/src/internal/handler/llm.goplatform-api/src/internal/handler/llm_template_integration_test.goplatform-api/src/internal/handler/parse_template_query_test.goplatform-api/src/internal/repository/interfaces.goplatform-api/src/internal/repository/llm.goplatform-api/src/internal/service/artifact_import_lifecycle_test.goplatform-api/src/internal/service/llm.goplatform-api/src/internal/service/llm_provider_template_test.goplatform-api/src/internal/utils/llm_provider_template_loader.goplatform-api/src/internal/utils/llm_provider_template_loader_test.goplatform-api/src/resources/default-llm-provider-templates/anthropic-template.yamlplatform-api/src/resources/default-llm-provider-templates/awsbedrock-template.yamlplatform-api/src/resources/default-llm-provider-templates/azureaifoundry-template.yamlplatform-api/src/resources/default-llm-provider-templates/azureopenai-template.yamlplatform-api/src/resources/default-llm-provider-templates/gemini-template.yamlplatform-api/src/resources/default-llm-provider-templates/mistral-template.yamlplatform-api/src/resources/default-llm-provider-templates/openai-template.yamlplatform-api/src/resources/openapi.yamlportals/ai-workspace/cypress/e2e/005-provider-templates/005-custom-provider-template.cy.jsportals/ai-workspace/src/apis/providerTemplateApis.tsportals/ai-workspace/src/contexts/llmProvider/providerTemplate/ProviderTemplateContext.tsxportals/ai-workspace/src/contexts/llmProvider/providerTemplate/ProviderTemplatesContext.tsxportals/ai-workspace/src/pages/appShell/appShellPages/providerTemplate/CreateProviderTemplateVersion.tsxportals/ai-workspace/src/pages/appShell/appShellPages/providerTemplate/ProviderTemplateOverview.tsxportals/ai-workspace/src/pages/appShell/appShellPages/providerTemplate/ProviderTemplatesList.tsxportals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/AddNewProvider/TemplateVersionDialog.tsxportals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/ServiceProviderNew.tsx
💤 Files with no reviewable changes (12)
- portals/ai-workspace/src/pages/appShell/appShellPages/providerTemplate/CreateProviderTemplateVersion.tsx
- portals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/AddNewProvider/TemplateVersionDialog.tsx
- platform-api/src/internal/service/artifact_import_lifecycle_test.go
- portals/ai-workspace/src/pages/appShell/appShellPages/providerTemplate/ProviderTemplatesList.tsx
- platform-api/src/internal/repository/interfaces.go
- portals/ai-workspace/src/contexts/llmProvider/providerTemplate/ProviderTemplateContext.tsx
- portals/ai-workspace/src/pages/appShell/appShellPages/providerTemplate/ProviderTemplateOverview.tsx
- portals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/ServiceProviderNew.tsx
- portals/ai-workspace/src/apis/providerTemplateApis.ts
- portals/ai-workspace/src/contexts/llmProvider/providerTemplate/ProviderTemplatesContext.tsx
- portals/ai-workspace/cypress/e2e/005-provider-templates/005-custom-provider-template.cy.js
- platform-api/src/resources/openapi.yaml
✅ Files skipped from review due to trivial changes (4)
- platform-api/src/resources/default-llm-provider-templates/awsbedrock-template.yaml
- platform-api/src/resources/default-llm-provider-templates/openai-template.yaml
- platform-api/src/resources/default-llm-provider-templates/anthropic-template.yaml
- platform-api/src/internal/constants/error.go
🚧 Files skipped from review as they are similar to previous changes (11)
- platform-api/src/resources/default-llm-provider-templates/gemini-template.yaml
- platform-api/src/resources/default-llm-provider-templates/mistral-template.yaml
- platform-api/src/internal/handler/parse_template_query_test.go
- platform-api/src/resources/default-llm-provider-templates/azureopenai-template.yaml
- platform-api/src/internal/utils/llm_provider_template_loader.go
- platform-api/src/internal/utils/llm_provider_template_loader_test.go
- platform-api/src/resources/default-llm-provider-templates/azureaifoundry-template.yaml
- platform-api/src/internal/handler/llm.go
- platform-api/src/internal/repository/llm.go
- platform-api/src/internal/service/llm.go
- platform-api/src/internal/service/llm_provider_template_test.go
Tharanidk
force-pushed
the
main-platform-portal
branch
2 times, most recently
from
bae7c90 to
0558daa
Compare
Tharanidk
force-pushed
the
main-platform-portal
branch
from
e91fbb7 to
e09f787
Compare
…s and update related code
Tharanidk
force-pushed
the
main-platform-portal
branch
from
e09f787 to
49937b4
Compare
…d name to displayName
Tharanidk
force-pushed
the
main-platform-portal
branch
from
49937b4 to
e72e0d5
Compare
Tharanidk
force-pushed
the
main-platform-portal
branch
from
657bc34 to
5eeae69
Compare
renuka-fernando
approved these changes
Jul 2, 2026
renuka-fernando
left a comment
renuka-fernando
left a comment
Contributor
There was a problem hiding this comment.
Let's do these changes in a separate PR
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
The handle should be changed to groupId with the change of some endpoints.
Approach
changed the handle to groupID
Changed the REST APIs as follows.