feat:add RAG service type — API design & validation#297
Open
tsivaprasad wants to merge 2 commits intomainfrom
Open
feat:add RAG service type — API design & validation#297tsivaprasad wants to merge 2 commits intomainfrom
tsivaprasad wants to merge 2 commits intomainfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds RAG service support: API schema accepts "rag"; adds RAG config parsing and validation; introduces recursive sensitive-data scrubbing; extensive RAG config tests; registers a RAG service image and adds an orchestrator guard that errors for non- Changes
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip CodeRabbit can scan for known vulnerabilities in your dependencies using OSV Scanner.OSV Scanner will automatically detect and report security vulnerabilities in your project's dependencies. No additional configuration is required. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
server/internal/orchestrator/swarm/orchestrator.go (1)
404-425: Consider moving the service type guard before image lookup.The guard at lines 422-425 runs after
GetServiceImage(line 406) andValidateCompatibility(line 413). Moving the type check earlier would fail faster for unsupported types without unnecessary image operations.♻️ Optional: Reorder for earlier failure
func (o *Orchestrator) GenerateServiceInstanceResources(spec *database.ServiceInstanceSpec) (*database.ServiceInstanceResources, error) { + // Only MCP service instance generation is currently implemented. + if spec.ServiceSpec.ServiceType != "mcp" { + return nil, fmt.Errorf("service type %q instance generation is not yet supported", spec.ServiceSpec.ServiceType) + } + // Get service image based on service type and version serviceImage, err := o.serviceVersions.GetServiceImage(spec.ServiceSpec.ServiceType, spec.ServiceSpec.Version) if err != nil { return nil, fmt.Errorf("failed to get service image: %w", err) } // Validate compatibility with database version if spec.PgEdgeVersion != nil { if err := serviceImage.ValidateCompatibility( spec.PgEdgeVersion.PostgresVersion, spec.PgEdgeVersion.SpockVersion, ); err != nil { return nil, fmt.Errorf("service %q version %q is not compatible with this database: %w", spec.ServiceSpec.ServiceType, spec.ServiceSpec.Version, err) } } - // Only MCP service instance generation is currently implemented. - if spec.ServiceSpec.ServiceType != "mcp" { - return nil, fmt.Errorf("service type %q instance generation is not yet supported", spec.ServiceSpec.ServiceType) - } - // Parse the MCP service config from the untyped config map🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/internal/orchestrator/swarm/orchestrator.go` around lines 404 - 425, In GenerateServiceInstanceResources, check the service type early and return for unsupported types before calling o.serviceVersions.GetServiceImage or invoking serviceImage.ValidateCompatibility; specifically, move the guard that inspects spec.ServiceSpec.ServiceType (the "mcp" check) to the top of the function (before GetServiceImage and ValidateCompatibility) so unsupported types short-circuit and avoid unnecessary image lookup and compatibility validation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@server/internal/orchestrator/swarm/orchestrator.go`:
- Around line 404-425: In GenerateServiceInstanceResources, check the service
type early and return for unsupported types before calling
o.serviceVersions.GetServiceImage or invoking
serviceImage.ValidateCompatibility; specifically, move the guard that inspects
spec.ServiceSpec.ServiceType (the "mcp" check) to the top of the function
(before GetServiceImage and ValidateCompatibility) so unsupported types
short-circuit and avoid unnecessary image lookup and compatibility validation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0307bfb7-2ce4-45cf-bc9b-0715cd8a396e
⛔ Files ignored due to path filters (6)
api/apiv1/gen/http/control_plane/client/types.gois excluded by!**/gen/**api/apiv1/gen/http/control_plane/server/types.gois excluded by!**/gen/**api/apiv1/gen/http/openapi.jsonis excluded by!**/gen/**api/apiv1/gen/http/openapi.yamlis excluded by!**/gen/**api/apiv1/gen/http/openapi3.jsonis excluded by!**/gen/**api/apiv1/gen/http/openapi3.yamlis excluded by!**/gen/**
📒 Files selected for processing (7)
api/apiv1/design/database.goserver/internal/api/apiv1/convert.goserver/internal/api/apiv1/validate.goserver/internal/database/rag_service_config.goserver/internal/database/rag_service_config_test.goserver/internal/orchestrator/swarm/orchestrator.goserver/internal/orchestrator/swarm/service_images.go
tsivaprasad
force-pushed
the
PLAT-488-rag-service-api-design-validation
branch
from
d56897d to
c6a2728
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/internal/orchestrator/swarm/orchestrator.go (1)
405-425:⚠️ Potential issue | 🟠 MajorMove unsupported-type gating before image/compatibility checks.
Line 406 and Lines 412-420 run before the unsupported-type guard on Line 423, so non-MCP requests can fail with image/compatibility errors instead of the intended explicit “not yet supported” error.
Proposed fix
func (o *Orchestrator) GenerateServiceInstanceResources(spec *database.ServiceInstanceSpec) (*database.ServiceInstanceResources, error) { + // Only MCP service instance generation is currently implemented. + if spec.ServiceSpec.ServiceType != "mcp" { + return nil, fmt.Errorf("service type %q instance generation is not yet supported", spec.ServiceSpec.ServiceType) + } + // Get service image based on service type and version serviceImage, err := o.serviceVersions.GetServiceImage(spec.ServiceSpec.ServiceType, spec.ServiceSpec.Version) if err != nil { return nil, fmt.Errorf("failed to get service image: %w", err) } @@ - // Only MCP service instance generation is currently implemented. - if spec.ServiceSpec.ServiceType != "mcp" { - return nil, fmt.Errorf("service type %q instance generation is not yet supported", spec.ServiceSpec.ServiceType) - } - // Parse the MCP service config from the untyped config map mcpConfig, errs := database.ParseMCPServiceConfig(spec.ServiceSpec.Config, false)As per coding guidelines: "Domain-specific errors should be defined in each package; API errors should be mapped to HTTP status codes via Goa".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/internal/orchestrator/swarm/orchestrator.go` around lines 405 - 425, Move the unsupported-type guard for spec.ServiceSpec.ServiceType != "mcp" to run before calling o.serviceVersions.GetServiceImage and before serviceImage.ValidateCompatibility so non-MCP requests immediately return the explicit "not yet supported" error instead of hitting image/compatibility paths; replace the generic fmt.Errorf with a package-level domain-specific error (e.g., ErrServiceTypeNotSupported) declared in this package and return that from the function so the API layer (Goa) can map it to the appropriate HTTP status code.
🧹 Nitpick comments (1)
server/internal/database/rag_service_config.go (1)
72-80: Make unknown-key error ordering deterministic.When multiple unknown top-level keys are present, map iteration order makes the joined message unstable. Sorting
unknownKeysbeforeJoinkeeps errors deterministic.Proposed refactor
if len(unknownKeys) > 0 { + slices.Sort(unknownKeys) errs = append(errs, fmt.Errorf("unknown config key(s): %s", strings.Join(unknownKeys, ", "))) return nil, errs }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/internal/database/rag_service_config.go` around lines 72 - 80, The unknownKeys slice collected from the config map should be sorted to make the error message deterministic; locate the block that builds unknownKeys (referencing ragKnownTopLevelKeys and the local variable unknownKeys) and, before calling strings.Join(unknownKeys, ", "), call sort.Strings(unknownKeys) (and add an import for "sort" if missing) so the appended error message produced for errs is stable across runs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@server/internal/orchestrator/swarm/orchestrator.go`:
- Around line 405-425: Move the unsupported-type guard for
spec.ServiceSpec.ServiceType != "mcp" to run before calling
o.serviceVersions.GetServiceImage and before serviceImage.ValidateCompatibility
so non-MCP requests immediately return the explicit "not yet supported" error
instead of hitting image/compatibility paths; replace the generic fmt.Errorf
with a package-level domain-specific error (e.g., ErrServiceTypeNotSupported)
declared in this package and return that from the function so the API layer
(Goa) can map it to the appropriate HTTP status code.
---
Nitpick comments:
In `@server/internal/database/rag_service_config.go`:
- Around line 72-80: The unknownKeys slice collected from the config map should
be sorted to make the error message deterministic; locate the block that builds
unknownKeys (referencing ragKnownTopLevelKeys and the local variable
unknownKeys) and, before calling strings.Join(unknownKeys, ", "), call
sort.Strings(unknownKeys) (and add an import for "sort" if missing) so the
appended error message produced for errs is stable across runs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d49275b6-5f8a-439c-baaf-91097adc1a04
⛔ Files ignored due to path filters (6)
api/apiv1/gen/http/control_plane/client/types.gois excluded by!**/gen/**api/apiv1/gen/http/control_plane/server/types.gois excluded by!**/gen/**api/apiv1/gen/http/openapi.jsonis excluded by!**/gen/**api/apiv1/gen/http/openapi.yamlis excluded by!**/gen/**api/apiv1/gen/http/openapi3.jsonis excluded by!**/gen/**api/apiv1/gen/http/openapi3.yamlis excluded by!**/gen/**
📒 Files selected for processing (7)
api/apiv1/design/database.goserver/internal/api/apiv1/convert.goserver/internal/api/apiv1/validate.goserver/internal/database/rag_service_config.goserver/internal/database/rag_service_config_test.goserver/internal/orchestrator/swarm/orchestrator.goserver/internal/orchestrator/swarm/service_images.go
🚧 Files skipped from review as they are similar to previous changes (3)
- api/apiv1/design/database.go
- server/internal/orchestrator/swarm/service_images.go
- server/internal/api/apiv1/convert.go
jason-lynch
approved these changes
Mar 19, 2026
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.
Summary
This PR adds
ragas a supported service type in the Control Plane API, including configuration parsing and validation for RAG pipeline definitions. It serves as the foundation for RAG service support, with instance provisioning to be introduced in a follow-up PR.Changes
ragto the service_type enum in the Goa design and regenerate all API/OpenAPI artifacts(pipelines, name, tables, embedding_llm, rag_llm), provider enums, API key requirements per provider (anthropic/openai/voyage require api_key;ollamadoes not), and numeric range checks (token_budget,top_n,search.vector_weight)scrubSensitiveConfighelper so nested sensitive keys (e.g.pipelines[].embedding_llm.api_key) are stripped from API read responsesrag-server:latestin ServiceVersions to enable version compatibility checksGenerateServiceInstanceResourcesto return a clear "not yet supported" error for non-MCP typesTesting
Unit tests (29 new RAG config tests)
go test ./server/internal/database/... -run TestParseRAG -vManual Verification:
restish control-plane-local-1 create-database < ../demo/488/rag_create_db.json[rag_create_db.json](https://github.com/user-attachments/files/25973502/rag_create_db.json)The database create request and the response was returned with HTTP 200 OK.
Verified that no sensitive keys are exposed in the API response.
host-1-1 | 2:28PM ERR failed to update database error={"error":"failed to execute plan update: failed to get service resources for rag on host-1: failed to generate service instance resources: service type \"rag\" instance generation is not yet supported","kind":"*fmt.wrapError","stack":null}Checklist
Notes for Reviewers
This PR intentionally does not provision a RAG service instance. Submitting with service_type: "rag" will pass API validation but fail at the workflow layer with "service type "rag" instance generation is not yet supported". This is by design for this ticket.
PLAT-488