Backend improvements#1857
Merged
Merged
Conversation
📝 WalkthroughWalkthroughThis PR adds a new ChangesAI Sample Rows Feature
Docker Compose Build Context Update
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant AgentsController
participant GetAiSampleRowsUseCase
participant DAO
Client->>AgentsController: POST /ai/data/:connectionId/sample-rows
AgentsController->>GetAiSampleRowsUseCase: execute(connectionId, userId, tableName, limit)
GetAiSampleRowsUseCase->>GetAiSampleRowsUseCase: verify read permission, derive readable columns
GetAiSampleRowsUseCase->>DAO: fetch first page (limit clamped to 5)
DAO-->>GetAiSampleRowsUseCase: rows, rowCount, largeDataset
GetAiSampleRowsUseCase-->>AgentsController: filtered AiSampleRowsRO
AgentsController-->>Client: 201 AiSampleRowsRO
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a new internal “AI sample rows” capability to the agents microservice (used for AI/website-generation flows), including end-to-end test coverage, and updates the local docker-compose setup for the private microservice build context.
Changes:
- Added
/internal/agents/ai/data/:connectionId/sample-rowsendpoint with DTO/DS/RO wiring and a dedicated use case that enforces table + column-level read permissions. - Registered the new use case in DI (token + module provider) and updated the agents use-case interface surface.
- Added an AVA e2e test validating success, limit clamping, missing microservice JWT (401), unauthorized user (403), and column whitelist filtering behavior.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
docker-compose.yml |
Updates build context/dockerfile/env_file paths for the private microservice to point at the sitenova monorepo layout. |
backend/test/ava-tests/non-saas-tests/non-saas-agents-microservice-sample-rows-e2e.test.ts |
New e2e tests for the internal sample-rows endpoint (auth, clamping, permissions, column filtering). |
backend/src/microservices/agents-microservice/use-cases/get-ai-sample-rows.use.case.ts |
Implements sample row retrieval + Cedar-based column filtering and max-limit clamping. |
backend/src/microservices/agents-microservice/use-cases/agents-use-cases.interface.ts |
Adds the IGetAiSampleRows use case interface and related DS/RO imports. |
backend/src/microservices/agents-microservice/dto/agents-ai-data.dtos.ts |
Adds GetAiSampleRowsDto (tableName + optional limit with validation). |
backend/src/microservices/agents-microservice/data-structures/agents.ds.ts |
Adds GetAiSampleRowsDs. |
backend/src/microservices/agents-microservice/data-structures/agents-responses.ds.ts |
Adds AiSampleRowsRO response model for Swagger/docs. |
backend/src/microservices/agents-microservice/agents.module.ts |
Registers the new use case provider under a new DI token. |
backend/src/microservices/agents-microservice/agents.controller.ts |
Adds the new POST route handler for sample rows. |
backend/src/common/data-injection.tokens.ts |
Introduces UseCaseType.AGENTS_GET_AI_SAMPLE_ROWS. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
docker-compose.yml (1)
107-113: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winImplicit sibling-repo dependency on
../sitenova.Build context, dockerfile, and env_file now all resolve outside this repository into a sibling
../sitenovacheckout. Any contributor or CI runner without that exact directory layout will fail to build/startrocketadmin-private-microservice. Consider documenting this monorepo-layout requirement (e.g., in README/CONTRIBUTING) or adding a setup script/CI step that ensures../sitenovais present, so the implicit contract isn't silently broken for new environments.🤖 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 `@docker-compose.yml` around lines 107 - 113, The docker-compose service for rocketadmin-private-microservice now depends on a sibling ../sitenova checkout for context, Dockerfile, and env_file, so make that contract explicit and enforced. Update the setup/docs path by referencing the compose service config and rocketadmin-saas Dockerfile.test usage, and add either README/CONTRIBUTING guidance or a setup/CI check that verifies ../sitenova exists before build/start. Keep the monorepo layout requirement discoverable for anyone running this service outside the current directory structure.backend/test/ava-tests/non-saas-tests/non-saas-agents-microservice-sample-rows-e2e.test.ts (1)
39-42: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse arrow functions instead of function declarations.
microserviceAuthHeader,userIdFromCookieToken,createConnectionAndTable, andsampleRowsRequestare all declared withfunction. As per coding guidelines, "Prefer arrow functions over function declarations" for**/*.{js,ts,jsx,tsx}files.♻️ Example conversion
-function microserviceAuthHeader(): string { +const microserviceAuthHeader = (): string => { const secret = appConfig.auth.microserviceJwtSecret as string; return `Bearer ${jwt.sign({ request_id: faker.string.uuid() }, secret, { expiresIn: '1h' })}`; -} +};Also applies to: 44-47, 83-112, 114-121
🤖 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 `@backend/test/ava-tests/non-saas-tests/non-saas-agents-microservice-sample-rows-e2e.test.ts` around lines 39 - 42, The test file uses function declarations where the code style requires arrow functions; update microserviceAuthHeader, userIdFromCookieToken, createConnectionAndTable, and sampleRowsRequest to arrow function expressions. Keep the existing behavior the same while converting each named function to an arrow function so the declarations align with the project’s TypeScript/JavaScript coding guidelines.Source: Coding guidelines
backend/src/microservices/agents-microservice/dto/agents-ai-data.dtos.ts (1)
42-46: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd
@Maxto match documented clamp.The Swagger description states the limit is "Clamped server-side to 5," but only
@Min(1)is enforced — there's no@Maxvalidator, so the documented contract isn't reflected in validation (values like 50 pass DTO validation and rely solely on use-case clamping). Not a functional bug since the use case clamps downstream, but the DTO's validation doesn't match its own documentation.♻️ Proposed fix
`@ApiPropertyOptional`({ description: 'Max sample rows to return. Clamped server-side to 5.' }) `@IsOptional`() `@IsInt`() `@Min`(1) + `@Max`(5) limit?: number;Also update the import:
-import { IsInt, IsNotEmpty, IsOptional, IsString, IsUUID, Min } from 'class-validator'; +import { IsInt, IsNotEmpty, IsOptional, IsString, IsUUID, Max, Min } from 'class-validator';🤖 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 `@backend/src/microservices/agents-microservice/dto/agents-ai-data.dtos.ts` around lines 42 - 46, The DTO field limit in agents-ai-data.dtos.ts documents a server-side clamp to 5, but the validation only enforces `@Min`(1), so the DTO contract doesn’t match the Swagger description. Update the limit property in the relevant DTO class to add `@Max`(5) alongside `@IsOptional`, `@IsInt`, and `@Min`(1), and make sure the validator import list includes Max so the validation rules and documentation stay aligned.
🤖 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 `@backend/src/microservices/agents-microservice/dto/agents-ai-data.dtos.ts`:
- Around line 42-46: The DTO field limit in agents-ai-data.dtos.ts documents a
server-side clamp to 5, but the validation only enforces `@Min`(1), so the DTO
contract doesn’t match the Swagger description. Update the limit property in the
relevant DTO class to add `@Max`(5) alongside `@IsOptional`, `@IsInt`, and `@Min`(1),
and make sure the validator import list includes Max so the validation rules and
documentation stay aligned.
In
`@backend/test/ava-tests/non-saas-tests/non-saas-agents-microservice-sample-rows-e2e.test.ts`:
- Around line 39-42: The test file uses function declarations where the code
style requires arrow functions; update microserviceAuthHeader,
userIdFromCookieToken, createConnectionAndTable, and sampleRowsRequest to arrow
function expressions. Keep the existing behavior the same while converting each
named function to an arrow function so the declarations align with the project’s
TypeScript/JavaScript coding guidelines.
In `@docker-compose.yml`:
- Around line 107-113: The docker-compose service for
rocketadmin-private-microservice now depends on a sibling ../sitenova checkout
for context, Dockerfile, and env_file, so make that contract explicit and
enforced. Update the setup/docs path by referencing the compose service config
and rocketadmin-saas Dockerfile.test usage, and add either README/CONTRIBUTING
guidance or a setup/CI check that verifies ../sitenova exists before
build/start. Keep the monorepo layout requirement discoverable for anyone
running this service outside the current directory structure.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 19edc4c7-252e-45fb-b8df-26552547d774
📒 Files selected for processing (10)
backend/src/common/data-injection.tokens.tsbackend/src/microservices/agents-microservice/agents.controller.tsbackend/src/microservices/agents-microservice/agents.module.tsbackend/src/microservices/agents-microservice/data-structures/agents-responses.ds.tsbackend/src/microservices/agents-microservice/data-structures/agents.ds.tsbackend/src/microservices/agents-microservice/dto/agents-ai-data.dtos.tsbackend/src/microservices/agents-microservice/use-cases/agents-use-cases.interface.tsbackend/src/microservices/agents-microservice/use-cases/get-ai-sample-rows.use.case.tsbackend/test/ava-tests/non-saas-tests/non-saas-agents-microservice-sample-rows-e2e.test.tsdocker-compose.yml
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
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 by CodeRabbit
New Features
Bug Fixes