-
Notifications
You must be signed in to change notification settings - Fork 0
✨ Feature: rud prompt #24
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
WalkthroughAdds prompt read/list/update/delete capabilities: new controller endpoints, service methods, DTOs for preview/detail/list and update, repository cursor pagination, entity mutability with update rules, and new error types (PromptException, CUSTOM_SYSTEM_PROMPT). Also makes create request’s systemPrompt optional and enforces preset-specific systemPrompt constraints. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant Controller as PromptController
participant Service as PromptService
participant Repo as PromptRepository
participant DB as Database
rect rgb(235,245,255)
note over Client,Controller: Read detail
Client->>Controller: GET /api/v1/prompts/{id}
Controller->>Service: getPrompt(id)
Service->>Repo: findByIdOrNull(id)
Repo-->>Service: Prompt?/null
alt found
Service-->>Controller: PromptDetailResponse
Controller-->>Client: 200 ApiResponse
else not found
Service-->>Controller: throws PromptException(NOT_FOUND)
Controller-->>Client: 404 ApiResponse
end
end
rect rgb(240,255,240)
note over Client,Controller: List (cursor)
Client->>Controller: GET /api/v1/prompts?cursor&take
Controller->>Service: getPrompts(cursor,take)
Service->>Repo: findByIdGreaterThanOrderById(cursor, PageRequest(0,take))
Repo-->>Service: Slice<Prompt>
Service-->>Controller: PromptPreviewResponseList
Controller-->>Client: 200 ApiResponse
end
rect rgb(255,245,235)
note over Client,Controller: Update
Client->>Controller: PUT /api/v1/prompts/{id} (PromptUpdateRequest)
Controller->>Service: updatePrompt(req,id)
Service->>Repo: findByIdOrNull(id)
Repo-->>Service: Prompt?/null
alt found
Service->>Service: prompt.update(req)
Service-->>Controller: PromptPreviewResponse
Controller-->>Client: 200 ApiResponse
else not found
Service-->>Controller: throws PromptException(NOT_FOUND)
Controller-->>Client: 404 ApiResponse
end
end
rect rgb(255,235,245)
note over Client,Controller: Delete
Client->>Controller: DELETE /api/v1/prompts/{id}
Controller->>Service: deletePrompt(id)
Service->>Repo: findByIdOrNull(id)
Repo-->>Service: Prompt?/null
alt found
Service->>Repo: delete(prompt)
Service-->>Controller: Unit
Controller-->>Client: 200 ApiResponse
else not found
Service-->>Controller: throws PromptException(NOT_FOUND)
Controller-->>Client: 404 ApiResponse
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 12
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
src/main/kotlin/simplerag/ragback/domain/prompt/controller/PromptController.kt(2 hunks)src/main/kotlin/simplerag/ragback/domain/prompt/dto/PromptRequestDTO.kt(1 hunks)src/main/kotlin/simplerag/ragback/domain/prompt/dto/PromptResponseDTO.kt(1 hunks)src/main/kotlin/simplerag/ragback/domain/prompt/entity/Prompt.kt(1 hunks)src/main/kotlin/simplerag/ragback/domain/prompt/repository/PromptRepository.kt(1 hunks)src/main/kotlin/simplerag/ragback/domain/prompt/service/PromptService.kt(2 hunks)src/main/kotlin/simplerag/ragback/global/error/CustomException.kt(1 hunks)src/main/kotlin/simplerag/ragback/global/error/ErrorCode.kt(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
src/main/kotlin/simplerag/ragback/domain/prompt/repository/PromptRepository.kt (1)
src/main/kotlin/simplerag/ragback/domain/document/repository/DataFileRepository.kt (1)
findByIdGreaterThanOrderById(11-11)
src/main/kotlin/simplerag/ragback/global/error/ErrorCode.kt (1)
src/main/kotlin/simplerag/ragback/domain/prompt/entity/enums/PreSet.kt (1)
description(3-65)
src/main/kotlin/simplerag/ragback/domain/prompt/controller/PromptController.kt (1)
src/main/kotlin/simplerag/ragback/domain/index/controller/IndexController.kt (1)
indexService(11-59)
src/main/kotlin/simplerag/ragback/global/error/CustomException.kt (1)
src/main/kotlin/simplerag/ragback/global/error/GlobalExceptionHandler.kt (3)
handleCustomException(80-86)handleFileException(72-78)log(16-95)
src/main/kotlin/simplerag/ragback/domain/prompt/dto/PromptResponseDTO.kt (1)
src/main/kotlin/simplerag/ragback/domain/prompt/entity/enums/PreSet.kt (1)
description(3-65)
src/main/kotlin/simplerag/ragback/domain/prompt/entity/Prompt.kt (1)
src/main/kotlin/simplerag/ragback/domain/prompt/entity/enums/PreSet.kt (1)
description(3-65)
🔇 Additional comments (8)
src/main/kotlin/simplerag/ragback/domain/prompt/dto/PromptRequestDTO.kt (1)
12-13: Kotlin 1.4+ Support Confirmed – No Action RequiredConfirmed that the project uses Kotlin plugin version 1.9.25, which fully supports trailing commas in primary constructors.
• Found in build.gradle (at lines 2, 3, and 6):
•id 'org.jetbrains.kotlin.jvm' version '1.9.25'
•id 'org.jetbrains.kotlin.plugin.spring' version '1.9.25'
•id 'org.jetbrains.kotlin.plugin.jpa' version '1.9.25'Trailing commas in the DTO constructor are safe under Kotlin 1.9.25.
src/main/kotlin/simplerag/ragback/domain/prompt/repository/PromptRepository.kt (1)
9-10: Next-cursor derivation verifiedThe
PromptService.getPrompts()implementation correctly computesnextCursorfrom the last element’sid(prompts.content.lastOrNull()?.id) and usesprompts.hasNext()solely to indicate further pages. No changes required.src/main/kotlin/simplerag/ragback/global/error/CustomException.kt (1)
25-28: LGTM: PromptException integrates cleanly with GlobalExceptionHandlerExtends CustomException with code-derived message; will be caught by the existing CustomException handler. No concerns.
src/main/kotlin/simplerag/ragback/domain/prompt/controller/PromptController.kt (2)
26-32: LGTM: Detail-read endpoint aligns with RUD objectivesSignature and return type (PromptDetailResponse) make sense for a detail view.
52-58: LGTM: Delete endpoint behavior consistent with existing APIReturns a success message via ApiResponse; consistent with IndexController’s pattern.
src/main/kotlin/simplerag/ragback/domain/prompt/dto/PromptResponseDTO.kt (1)
36-47: Detail DTO looks good and aligns with service usage.Fields are appropriate for "detail read" and consistent with entity. Returning the enum is fine (serializes as string). No issues.
src/main/kotlin/simplerag/ragback/domain/prompt/entity/Prompt.kt (1)
16-24: Making fields mutable is necessary for updates.Switching name, preSet, and systemPrompt to var is appropriate for in-place JPA updates.
src/main/kotlin/simplerag/ragback/domain/prompt/service/PromptService.kt (1)
30-33: LGTM: getPrompt aligns with NOT_FOUND handling and DTO mapping.Implementation is straightforward and correct.
| @GetMapping | ||
| fun getPrompts( | ||
| @RequestParam cursor: Long, | ||
| @RequestParam take: Int, | ||
| ): ApiResponse<PromptPreviewResponseList> { | ||
| val promptPreviewResponseList = promptService.getPrompts(cursor, take) | ||
| return ApiResponse.ok(promptPreviewResponseList) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add sane defaults and validation for cursor/take; enable @validated
Without defaults, the list endpoint 400s unless both params are provided. Also guard against negative cursor and unbounded take. You’ll need @validated on the controller class for @Min/@max on @RequestParam to take effect.
Apply this diff to the parameter annotations:
- fun getPrompts(
- @RequestParam cursor: Long,
- @RequestParam take: Int,
- ): ApiResponse<PromptPreviewResponseList> {
+ fun getPrompts(
+ @RequestParam(defaultValue = "0") @jakarta.validation.constraints.Min(0) cursor: Long,
+ @RequestParam(defaultValue = "20") @jakarta.validation.constraints.Min(1) @jakarta.validation.constraints.Max(100) take: Int,
+ ): ApiResponse<PromptPreviewResponseList> {Add @validated on the controller (outside selected range) so the @RequestParam constraints are enforced:
import org.springframework.validation.annotation.Validated
@Validated
@RestController
@RequestMapping("/api/v1/prompts")
class PromptController(
private val promptService: PromptService
) { ... }| @PutMapping("/{promptId}") | ||
| fun updatePrompt( | ||
| @RequestBody @Valid promptUpdateRequest: PromptUpdateRequest, | ||
| @PathVariable promptId: Long, | ||
| ): ApiResponse<PromptPreviewResponse> { | ||
| val updatedPrompt = promptService.updatePrompt(promptUpdateRequest, promptId) | ||
| return ApiResponse.ok(updatedPrompt) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Confirm update rules for non-CUSTOM presets and test them
Ensure the service/domain layer:
- rejects systemPrompt changes when preSet != CUSTOM (throw PromptException with CUSTOM_SYSTEM_PROMPT), and
- sets/retains the correct defaultSystemPrompt when switching presets.
Add integration tests covering:
- CUSTOM → non-CUSTOM with systemPrompt provided (expect 400/PROMPT_001),
- non-CUSTOM → CUSTOM without systemPrompt (expect default or explicit rule),
- simple name-only updates.
I can generate test scaffolding for these scenarios if helpful.
| data class PromptUpdateRequest( | ||
| @field:Size(max = 100) | ||
| val name: String, | ||
| val preSet: PreSet, | ||
| @field:Size(max = 20000) | ||
| val systemPrompt: String?, | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Harden request validation and keep DB length aligned
- Consider enforcing non-blank names on update as well. Today an empty string passes @SiZe(max=100).
- Verify the DB column length for systemPrompt matches 20,000 to avoid silent truncation or persistence errors.
Apply this diff to enforce non-blank names on update:
- @field:Size(max = 100)
- val name: String,
+ @field:NotBlank
+ @field:Size(max = 100)
+ val name: String,Add the missing import (outside the selected range):
import jakarta.validation.constraints.NotBlankVerify DB mapping length matches:
Also consider applying @notblank to PromptCreateRequest.name for parity with update.
🏁 Script executed:
#!/bin/bash
# Find the Prompt entity and check 'systemPrompt' column length mapping
rg -nP --type=kt -C3 '\bclass\s+Prompt\b|@Column|\bsystemPrompt\b' src/main/kotlinLength of output: 111
I’ll re-run the validation search with a glob-based grep to catch all Kotlin files:
This will confirm whether the DB column mapping for systemPrompt actually sets a 20,000-character length.
🏁 Script executed:
#!/bin/bash
# Search for Prompt entity definition
rg -n -C3 -g '*.kt' 'class Prompt' src/main/kotlin
# Look for the systemPrompt field in Prompt
rg -n -C3 -g '*.kt' 'systemPrompt' src/main/kotlin
# Check any @Column annotations specifying length
rg -n -C3 -g '*.kt' '@Column.*length' src/main/kotlinLength of output: 18126
Enforce non-blank names on both create/update and clarify systemPrompt storage
-
PromptRequestDTO.kt
• Add@field:NotBlankabovenamein bothPromptCreateRequestandPromptUpdateRequestto prevent empty strings
• Include the import:import jakarta.validation.constraints.NotBlank
• Keep the existing
@field:Size(max = 100)to enforce the length limit -
Prompt entity mapping (src/main/kotlin/simplerag/ragback/domain/prompt/entity/Prompt.kt)
• Currently,systemPromptis annotated with@Lob(no explicit length), so it’s stored as a CLOB with effectively unlimited size
• If you want to cap at 20,000 characters instead, replace:@Column(name = "system_prompt", nullable = false) @Lob var systemPrompt: String,
with:
@Column(name = "system_prompt", length = 20000, nullable = false) var systemPrompt: String,
(and remove
@Lob)
• Otherwise, confirm your database’s CLOB type supports your required maximum size
| data class PromptPreviewResponseList( | ||
| val promptPreviewResponseList: List<PromptPreviewResponse>, | ||
| val cursor: Long?, | ||
| val hasNext: Boolean | ||
| ) { | ||
| companion object { | ||
| fun from(prompts: List<Prompt>, cursor: Long?, hasNext: Boolean): PromptPreviewResponseList = | ||
| PromptPreviewResponseList( | ||
| promptPreviewResponseList = prompts.map { prompt -> | ||
| PromptPreviewResponse.from(prompt) | ||
| }, | ||
| cursor = cursor, | ||
| hasNext = hasNext, | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Prefer explicit nextCursor naming and simpler list field; also simplify mapping.
"cursor" here represents the next cursor the client should use, not the current one. Rename it to nextCursor to avoid API ambiguity. Also, "promptPreviewResponseList" is verbose—"items" reads better. Lastly, the mapping can be simplified.
This is an API surface change; confirm consumers before applying.
data class PromptPreviewResponseList(
- val promptPreviewResponseList: List<PromptPreviewResponse>,
- val cursor: Long?,
+ val items: List<PromptPreviewResponse>,
+ val nextCursor: Long?,
val hasNext: Boolean
) {
companion object {
- fun from(prompts: List<Prompt>, cursor: Long?, hasNext: Boolean): PromptPreviewResponseList =
+ fun from(prompts: List<Prompt>, nextCursor: Long?, hasNext: Boolean): PromptPreviewResponseList =
PromptPreviewResponseList(
- promptPreviewResponseList = prompts.map { prompt ->
- PromptPreviewResponse.from(prompt)
- },
- cursor = cursor,
+ items = prompts.map(PromptPreviewResponse::from),
+ nextCursor = nextCursor,
hasNext = hasNext,
)
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| data class PromptPreviewResponseList( | |
| val promptPreviewResponseList: List<PromptPreviewResponse>, | |
| val cursor: Long?, | |
| val hasNext: Boolean | |
| ) { | |
| companion object { | |
| fun from(prompts: List<Prompt>, cursor: Long?, hasNext: Boolean): PromptPreviewResponseList = | |
| PromptPreviewResponseList( | |
| promptPreviewResponseList = prompts.map { prompt -> | |
| PromptPreviewResponse.from(prompt) | |
| }, | |
| cursor = cursor, | |
| hasNext = hasNext, | |
| ) | |
| } | |
| } | |
| data class PromptPreviewResponseList( | |
| val items: List<PromptPreviewResponse>, | |
| val nextCursor: Long?, | |
| val hasNext: Boolean | |
| ) { | |
| companion object { | |
| fun from(prompts: List<Prompt>, nextCursor: Long?, hasNext: Boolean): PromptPreviewResponseList = | |
| PromptPreviewResponseList( | |
| items = prompts.map(PromptPreviewResponse::from), | |
| nextCursor = nextCursor, | |
| hasNext = hasNext, | |
| ) | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/main/kotlin/simplerag/ragback/domain/prompt/dto/PromptResponseDTO.kt
around lines 6-21, rename the "cursor" property to "nextCursor" and the
"promptPreviewResponseList" field to the shorter "items", update the companion
factory signature to from(prompts: List<Prompt>, nextCursor: Long?, hasNext:
Boolean), simplify the mapping to prompts.map(PromptPreviewResponse::from) when
building the DTO, and update all call sites/consumers to the new property names
(confirm consumers before applying this API change).
| data class PromptPreviewResponse( | ||
| val id: Long, | ||
| val name: String, | ||
| ) { | ||
| companion object { | ||
| fun from( | ||
| prompt: Prompt | ||
| ): PromptPreviewResponse { | ||
| return PromptPreviewResponse(prompt.id) | ||
| return PromptPreviewResponse(prompt.id, prompt.name) | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Use expression body for compact mapping.
Minor readability nit: expression body is concise and idiomatic.
companion object {
- fun from(
- prompt: Prompt
- ): PromptPreviewResponse {
- return PromptPreviewResponse(prompt.id, prompt.name)
- }
+ fun from(prompt: Prompt): PromptPreviewResponse =
+ PromptPreviewResponse(prompt.id, prompt.name)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| data class PromptPreviewResponse( | |
| val id: Long, | |
| val name: String, | |
| ) { | |
| companion object { | |
| fun from( | |
| prompt: Prompt | |
| ): PromptPreviewResponse { | |
| return PromptPreviewResponse(prompt.id) | |
| return PromptPreviewResponse(prompt.id, prompt.name) | |
| } | |
| } | |
| data class PromptPreviewResponse( | |
| val id: Long, | |
| val name: String, | |
| ) { | |
| companion object { | |
| fun from(prompt: Prompt): PromptPreviewResponse = | |
| PromptPreviewResponse(prompt.id, prompt.name) | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/main/kotlin/simplerag/ragback/domain/prompt/dto/PromptResponseDTO.kt
around lines 23 to 33, the companion object's from(...) function uses a block
body returning a new instance; change it to an expression body for conciseness
and idiomatic Kotlin by replacing the function block with a single-expression
implementation that directly returns PromptPreviewResponse(prompt.id,
prompt.name).
| fun update(promptUpdateRequest: PromptUpdateRequest) { | ||
| when (promptUpdateRequest.preSet) { | ||
| PreSet.CUSTOM -> { | ||
| this.name = promptUpdateRequest.name | ||
| this.systemPrompt = promptUpdateRequest.systemPrompt ?: "" | ||
| this.preSet = promptUpdateRequest.preSet | ||
| } | ||
| else -> { | ||
| if(!promptUpdateRequest.systemPrompt.isNullOrBlank()) { | ||
| throw PromptException(ErrorCode.CUSTOM_SYSTEM_PROMPT) | ||
| } | ||
| this.name = promptUpdateRequest.name | ||
| this.systemPrompt = promptUpdateRequest.preSet.defaultSystemPrompt | ||
| this.preSet = promptUpdateRequest.preSet | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Normalize inputs on update; enforce same rules with trimmed values.
- Trim name to prevent accidental whitespace persistence.
- Trim systemPrompt for CUSTOM before storing.
PreSet.CUSTOM -> {
- this.name = promptUpdateRequest.name
- this.systemPrompt = promptUpdateRequest.systemPrompt ?: ""
+ this.name = promptUpdateRequest.name.trim()
+ this.systemPrompt = promptUpdateRequest.systemPrompt?.trim().orEmpty()
this.preSet = promptUpdateRequest.preSet
}
else -> {
if(!promptUpdateRequest.systemPrompt.isNullOrBlank()) {
throw PromptException(ErrorCode.CUSTOM_SYSTEM_PROMPT)
}
- this.name = promptUpdateRequest.name
+ this.name = promptUpdateRequest.name.trim()
this.systemPrompt = promptUpdateRequest.preSet.defaultSystemPrompt
this.preSet = promptUpdateRequest.preSet
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fun update(promptUpdateRequest: PromptUpdateRequest) { | |
| when (promptUpdateRequest.preSet) { | |
| PreSet.CUSTOM -> { | |
| this.name = promptUpdateRequest.name | |
| this.systemPrompt = promptUpdateRequest.systemPrompt ?: "" | |
| this.preSet = promptUpdateRequest.preSet | |
| } | |
| else -> { | |
| if(!promptUpdateRequest.systemPrompt.isNullOrBlank()) { | |
| throw PromptException(ErrorCode.CUSTOM_SYSTEM_PROMPT) | |
| } | |
| this.name = promptUpdateRequest.name | |
| this.systemPrompt = promptUpdateRequest.preSet.defaultSystemPrompt | |
| this.preSet = promptUpdateRequest.preSet | |
| } | |
| fun update(promptUpdateRequest: PromptUpdateRequest) { | |
| when (promptUpdateRequest.preSet) { | |
| PreSet.CUSTOM -> { | |
| this.name = promptUpdateRequest.name.trim() | |
| this.systemPrompt = promptUpdateRequest.systemPrompt?.trim().orEmpty() | |
| this.preSet = promptUpdateRequest.preSet | |
| } | |
| else -> { | |
| if (!promptUpdateRequest.systemPrompt.isNullOrBlank()) { | |
| throw PromptException(ErrorCode.CUSTOM_SYSTEM_PROMPT) | |
| } | |
| this.name = promptUpdateRequest.name.trim() | |
| this.systemPrompt = promptUpdateRequest.preSet.defaultSystemPrompt | |
| this.preSet = promptUpdateRequest.preSet | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/main/kotlin/simplerag/ragback/domain/prompt/entity/Prompt.kt around lines
52 to 66, inputs are not normalized on update: trim the incoming
promptUpdateRequest.name and promptUpdateRequest.systemPrompt before validation
and assignment. Specifically, for PreSet.CUSTOM trim the name and trim
systemPrompt (use empty string if null after trim) before storing; for
non-CUSTOM, perform the isNullOrBlank check on the trimmed systemPrompt and
assign name and systemPrompt using trimmed values (use
preSet.defaultSystemPrompt for systemPrompt when appropriate) and set preSet as
before.
| fun getPrompts( | ||
| cursor: Long, | ||
| take: Int | ||
| ): PromptPreviewResponseList { | ||
| val prompts = promptRepository.findByIdGreaterThanOrderById(cursor, PageRequest.of(0, take)) | ||
|
|
||
| val nextCursor = prompts.content.lastOrNull()?.id | ||
|
|
||
| return PromptPreviewResponseList.from(prompts.content, nextCursor, prompts.hasNext()) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Harden paging inputs: clamp take and prevent negative cursors.
Avoid accidental large page sizes and negative cursors that could stress DB or yield confusing results. Also aligns with cursor semantics used in the DTO (nextCursor).
fun getPrompts(
cursor: Long,
take: Int
): PromptPreviewResponseList {
- val prompts = promptRepository.findByIdGreaterThanOrderById(cursor, PageRequest.of(0, take))
+ val safeTake = take.coerceIn(1, 100)
+ val safeCursor = cursor.coerceAtLeast(0L)
+ val prompts = promptRepository.findByIdGreaterThanOrderById(
+ safeCursor,
+ PageRequest.of(0, safeTake)
+ )
val nextCursor = prompts.content.lastOrNull()?.id
return PromptPreviewResponseList.from(prompts.content, nextCursor, prompts.hasNext())
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fun getPrompts( | |
| cursor: Long, | |
| take: Int | |
| ): PromptPreviewResponseList { | |
| val prompts = promptRepository.findByIdGreaterThanOrderById(cursor, PageRequest.of(0, take)) | |
| val nextCursor = prompts.content.lastOrNull()?.id | |
| return PromptPreviewResponseList.from(prompts.content, nextCursor, prompts.hasNext()) | |
| } | |
| fun getPrompts( | |
| cursor: Long, | |
| take: Int | |
| ): PromptPreviewResponseList { | |
| val safeTake = take.coerceIn(1, 100) | |
| val safeCursor = cursor.coerceAtLeast(0L) | |
| val prompts = promptRepository.findByIdGreaterThanOrderById( | |
| safeCursor, | |
| PageRequest.of(0, safeTake) | |
| ) | |
| val nextCursor = prompts.content.lastOrNull()?.id | |
| return PromptPreviewResponseList.from(prompts.content, nextCursor, prompts.hasNext()) | |
| } |
🤖 Prompt for AI Agents
In src/main/kotlin/simplerag/ragback/domain/prompt/service/PromptService.kt
around lines 35 to 44, the paging inputs are not validated: clamp the requested
take to a safe min/max (e.g. at least 1 and a reasonable max like 100) before
creating the PageRequest, and ensure cursor is non-negative (treat negative
cursor as 0 or throw IllegalArgumentException). Modify the method to sanitize
cursor and take up-front, then call
promptRepository.findByIdGreaterThanOrderById with PageRequest.of(0,
sanitizedTake) and compute nextCursor as before.
| @Transactional | ||
| fun updatePrompt( | ||
| promptUpdateRequest: PromptUpdateRequest, | ||
| promptId: Long | ||
| ): PromptPreviewResponse { | ||
| val prompt = promptRepository.findByIdOrNull(promptId) ?: throw PromptException(ErrorCode.NOT_FOUND) | ||
| prompt.update(promptUpdateRequest) | ||
| return PromptPreviewResponse.from(prompt) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Reduce repetition: extract a find-or-throw helper.
You repeat the NOT_FOUND lookup pattern. Centralize it for clarity and consistency.
- val prompt = promptRepository.findByIdOrNull(promptId) ?: throw PromptException(ErrorCode.NOT_FOUND)
+ val prompt = findPromptOrThrow(promptId)
prompt.update(promptUpdateRequest)
return PromptPreviewResponse.from(prompt)Add this helper (outside the shown ranges):
private fun findPromptOrThrow(id: Long): Prompt =
promptRepository.findByIdOrNull(id) ?: throw PromptException(ErrorCode.NOT_FOUND)🤖 Prompt for AI Agents
In src/main/kotlin/simplerag/ragback/domain/prompt/service/PromptService.kt
around lines 46 to 54, you're repeating the "find by id or throw NOT_FOUND"
pattern; add a private helper method named findPromptOrThrow that looks up a
Prompt by id via promptRepository.findByIdOrNull and throws
PromptException(ErrorCode.NOT_FOUND) when null, then replace the inline lookup
in updatePrompt (and any other occurrences) to call this helper so the lookup
and exception logic is centralized and consistent.
| @Transactional | ||
| fun deletePrompt(promptId: Long) { | ||
| val prompt = promptRepository.findByIdOrNull(promptId) ?: throw PromptException(ErrorCode.NOT_FOUND) | ||
| promptRepository.delete(prompt) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Decide on DELETE semantics: strict 404 vs idempotent delete.
Current behavior throws NOT_FOUND if the prompt doesn't exist. Many APIs choose idempotent deletes (204 even if absent) to reduce information leakage and simplify clients. If you prefer idempotency, consider no-op on missing id.
🤖 Prompt for AI Agents
In src/main/kotlin/simplerag/ragback/domain/prompt/service/PromptService.kt
around lines 56 to 60, the deletePrompt currently throws a NOT_FOUND
PromptException when the prompt is missing; decide on DELETE semantics and
implement idempotent delete: change the method to be a no-op when the prompt
doesn't exist (do not throw), e.g., check repository existence and return
silently or use deleteById without raising if absent, ensuring callers receive
success for repeated deletes; if you prefer strict behavior instead, keep the
current exception but update docs/tests to assert 404 on missing resources.
| OVERLAP_OVERFLOW(HttpStatus.BAD_REQUEST, "INDEX_001", "overlap 크기는 chunking 크기를 넘을 수 없습니다."), | ||
|
|
||
| // prompt | ||
| CUSTOM_SYSTEM_PROMPT(HttpStatus.BAD_REQUEST, "PROMPT_001", "preset이 Custom이 아닐 때 systemPrompt의 조작이 불가능합니다.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Nit: Keep preset name casing consistent with enum value
Use “CUSTOM” (enum name) instead of “Custom” in the message for consistency and easier grep/search.
Apply this diff:
- CUSTOM_SYSTEM_PROMPT(HttpStatus.BAD_REQUEST, "PROMPT_001", "preset이 Custom이 아닐 때 systemPrompt의 조작이 불가능합니다.")
+ CUSTOM_SYSTEM_PROMPT(HttpStatus.BAD_REQUEST, "PROMPT_001", "preset이 CUSTOM이 아닐 때 systemPrompt의 조작이 불가능합니다.")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| OVERLAP_OVERFLOW(HttpStatus.BAD_REQUEST, "INDEX_001", "overlap 크기는 chunking 크기를 넘을 수 없습니다."), | |
| // prompt | |
| CUSTOM_SYSTEM_PROMPT(HttpStatus.BAD_REQUEST, "PROMPT_001", "preset이 Custom이 아닐 때 systemPrompt의 조작이 불가능합니다.") | |
| OVERLAP_OVERFLOW(HttpStatus.BAD_REQUEST, "INDEX_001", "overlap 크기는 chunking 크기를 넘을 수 없습니다."), | |
| // prompt | |
| CUSTOM_SYSTEM_PROMPT(HttpStatus.BAD_REQUEST, "PROMPT_001", "preset이 CUSTOM이 아닐 때 systemPrompt의 조작이 불가능합니다.") |
🤖 Prompt for AI Agents
In src/main/kotlin/simplerag/ragback/global/error/ErrorCode.kt around lines 28
to 31, the error message for CUSTOM_SYSTEM_PROMPT uses "Custom" while the enum
name is CUSTOM; update the message string to use "CUSTOM" (uppercase) so casing
matches the enum and is easier to grep, leaving the rest of the enum entry
unchanged.
📌 Overview
rud prompt
🔍 Related Issues
✨ Changes
✨ Feature: get detail prompt
✨ Feature: get prompt
✨ Feature: update prompt
✨ Feature: delete prompt
📸 Screenshots / Test Results (Optional)
Attach images or videos if necessary.
✅ Checklist
🗒️ Additional Notes
Add any other context or information here.
Summary by CodeRabbit
New Features
Improvements