[ docs ] : 아키텍처 준수 검증 스킬(architecture-skill) 추가#32
Conversation
📝 WalkthroughWalkthroughChangesThe PR adds architecture verification skills and rules, reorganizes DTOs by domain and request/response role, splits session and user orchestration into usecases and focused services, updates transactional boundaries, and adjusts controllers, error handling, and tests accordingly. Architecture and contracts
Session usecase restructuring
User and onboarding usecases
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant SessionController
participant SessionLessonUsecase
participant SessionQueryService
participant CurriculumService
Client->>SessionController: Request lesson or reaction
SessionController->>SessionLessonUsecase: getLesson or getReaction
SessionLessonUsecase->>SessionQueryService: Load started session and snapshot
SessionLessonUsecase->>CurriculumService: Load lesson content by phase
CurriculumService-->>SessionLessonUsecase: Return question and hint note
SessionLessonUsecase-->>SessionController: Return session lesson response
SessionController-->>Client: HTTP response
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/main/kotlin/org/prography/samsung/backend/session/service/SessionCompletionService.kt (2)
21-27: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftMove cross-domain reward updates behind domain services
UserProfileRepository,UserCurriculumRepository, andBadgeLevelRepositorylive outside the session domain, soSessionCompletionServiceis now handling user/gamification mutations directly. KeepCoinLedgerEntryRepositoryhere, but move the profile/curriculum progress update and badge selection behind the owning service(s) instead of injecting those repositories into session.🤖 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 `@src/main/kotlin/org/prography/samsung/backend/session/service/SessionCompletionService.kt` around lines 21 - 27, Remove direct UserProfileRepository, UserCurriculumRepository, and BadgeLevelRepository dependencies from SessionCompletionService, and delegate profile/curriculum progress updates and badge selection to the owning domain service(s). Keep CoinLedgerEntryRepository in the session service and preserve the existing completion flow through service-level operations.
79-88: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftMake the session reward path atomic
existsBySessionId()is still a check-then-act gap here, and the completion flow has no transaction boundary. Two concurrentcomplete()calls can both apply the profile reward before any later session conflict, so one session can still mint coins twice. Add an atomic guard at the persistence layer and commit the whole flow as one unit.🤖 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 `@src/main/kotlin/org/prography/samsung/backend/session/service/SessionCompletionService.kt` around lines 79 - 88, The session reward logic in complete() is vulnerable to concurrent duplicate rewards because existsBySessionId() and save() are not atomic and the flow lacks a transaction boundary. Add a persistence-level uniqueness/atomic guard for the session reward, make complete() transactional so profile updates and CoinLedgerEntry persistence commit as one unit, and handle duplicate attempts without awarding coins twice.src/test/kotlin/org/prography/samsung/backend/session/service/SessionServiceTest.kt (1)
126-126: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winAdd the pending-reward
getStatuscase toSessionQueryServiceTest.
The미수령 보상 세션coverage was removed fromSessionServiceTest, and there’s no matching test inSessionQueryServiceTest. Add the equivalent assertion there to keepgetStatuscovered.🤖 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 `@src/test/kotlin/org/prography/samsung/backend/session/service/SessionServiceTest.kt` at line 126, Extend SessionQueryServiceTest with the pending-reward (“미수령 보상 세션”) getStatus scenario removed from SessionServiceTest. Recreate the equivalent setup and assertion for the pending-reward session, ensuring the test verifies the expected getStatus result.
🧹 Nitpick comments (5)
.agents/skills/architecture-skill/SKILL.md (1)
33-41: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider specifying a language for the fenced code block.
The code block starting at line 33 has no language specifier, triggering markdownlint MD040. Adding
textas the language keeps it informative for syntax highlighters.♻️ Optional fix
`
-+text
.../{domain}/controller/X.kt → (domain, controller)</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In @.agents/skills/architecture-skill/SKILL.md around lines 33 - 41, Update the
fenced code block containing the architecture path mappings in SKILL.md to
specify the text language, changing the opening fence to use text while
preserving its contents.</details> <!-- cr-comment:v1:d72c9eca2f05b6e52c5900c4 --> _Source: Linters/SAST tools_ </blockquote></details> <details> <summary>.agents/skills/architecture-skill/report-template.md (1)</summary><blockquote> `5-5`: _📐 Maintainability & Code Quality_ | _🔵 Trivial_ | _💤 Low value_ **Consider specifying a language for the fenced code block.** The code block starting at line 5 has no language specifier (markdownlint MD040). Since it contains Markdown template content, `markdown` would be the appropriate language. <details> <summary>♻️ Optional fix</summary> ```diff -``` +```markdown ## 🏛 아키텍처 검증 리포트🤖 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 @.agents/skills/architecture-skill/report-template.md at line 5, Update the fenced code block in the report template’s Markdown content to include the `markdown` language specifier, preserving the existing template content unchanged.Source: Linters/SAST tools
src/main/kotlin/org/prography/samsung/backend/gamification/dto/response/RewardResponse.kt (1)
18-31: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting the
from()mapping logic to a dedicated mapper.
RewardResponse.from()imports entities from three domains (session,user,gamification), coupling a gamification DTO to session and user entities. The project already uses a separateHintNoteMapperpattern in the curriculum domain. Moving this mapping to a dedicated mapper (e.g.,RewardMapperin the session or gamification service layer) would keep the DTO pure and centralize cross-domain coordination where it's architecturally appropriate.This is not blocking — the factory pattern is idiomatic Kotlin and the logic itself is correct.
🤖 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 `@src/main/kotlin/org/prography/samsung/backend/gamification/dto/response/RewardResponse.kt` around lines 18 - 31, Extract the mapping logic from RewardResponse.from into a dedicated RewardMapper, following the existing HintNoteMapper pattern and placing it in the appropriate session or gamification service layer. Move the session, profile, badgeLevelUp, and newBadge mapping there, then remove the DTO factory and update callers to use the mapper while preserving the current defaults and response values.src/main/kotlin/org/prography/samsung/backend/user/dto/request/UserSettingsRequest.kt (1)
22-27: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueAlign the
@Schematime pattern with the actual schedule rule. The current regex documents00:00–23:00, but the schedule validator only accepts15:00–20:00on the hour.🤖 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 `@src/main/kotlin/org/prography/samsung/backend/user/dto/request/UserSettingsRequest.kt` around lines 22 - 27, The `@Schema` pattern on UserSettingsRequest.time currently permits every hour, while schedule validation only accepts 15:00 through 20:00. Update the pattern to document exactly those allowed hourly values, preserving the nullable field and existing example.src/main/kotlin/org/prography/samsung/backend/user/service/UserProfileService.kt (1)
22-26: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsolidate duplicated profile lookup.
getUserProfileResponsedirectly callsuserProfileRepository.findById(userId).orElseThrow { ... }(lines 23-26), duplicating the exact logic ingetUserProfile(lines 43-45). Use the helper to keep error handling consistent and reduce duplication.♻️ Proposed refactor
fun getUserProfileResponse(userId: Long): UserProfileResponse { val profile = - userProfileRepository.findById(userId).orElseThrow { - CustomException(DomainErrorCode.USER_NOT_FOUND) - } + getUserProfile(userId) val userCurriculum = getUserCurriculum(userId)Also applies to: 43-45
🤖 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 `@src/main/kotlin/org/prography/samsung/backend/user/service/UserProfileService.kt` around lines 22 - 26, Update getUserProfileResponse to obtain the profile through the existing getUserProfile helper instead of directly calling userProfileRepository.findById(...).orElseThrow(...). Preserve the current USER_NOT_FOUND behavior and leave the helper as the single profile-lookup implementation.
🤖 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
`@src/main/kotlin/org/prography/samsung/backend/curriculum/dto/response/LessonContent.kt`:
- Around line 1-3: Rename the outbound DTO class LessonContent to
LessonContentResponse, preserving its constructor properties and data-class
behavior, and update all references including
CurriculumService.getLessonContent() and its callers to use the new symbol.
In
`@src/main/kotlin/org/prography/samsung/backend/curriculum/service/CurriculumService.kt`:
- Around line 64-79: Update the null-handling in
CurriculumService.getLessonContent to throw a dedicated question-not-found error
instead of DomainErrorCode.LESSON_TOPIC_NOT_FOUND. Add a unique
LESSON_QUESTION_NOT_FOUND entry to DomainErrorCode, using the project’s existing
error-code conventions, and reference it when
lessonQuestionRepository.findByLessonTopicIdAndPhase returns null; preserve the
existing hint-note error handling.
In
`@src/main/kotlin/org/prography/samsung/backend/session/service/SessionCompletionService.kt`:
- Around line 34-36: Update the completed-session branch in
SessionCompletionService to replace the no-argument orElseThrow on
userProfileRepository.findById(userId) with the same
CustomException(DomainErrorCode.USER_NOT_FOUND) supplier used in the normal
path, preserving the RewardResponse behavior when the profile exists.
In
`@src/main/kotlin/org/prography/samsung/backend/session/service/SessionLifecycleService.kt`:
- Around line 33-84: Enforce at the persistence layer that each user can have at
most one TutoringSession with SessionStatus.STARTED, using a database unique
constraint or equivalent atomic locking/conditional insert. Update the
repository/entity configuration around start and findByUserIdAndStatus so
concurrent SessionLifecycleUsecase.start requests cannot create duplicate active
sessions, while preserving the existing resume behavior.
In
`@src/main/kotlin/org/prography/samsung/backend/session/usecase/SessionLifeCycleUsecase.kt`:
- Around line 14-15: Rename SessionLifeCycleUsecase to SessionLifecycleUsecase,
including the Kotlin file, class declaration, and all references. Update the
import and field usage in SessionController to use the consistently spelled
symbol.
- Around line 20-27: Remove the local targetCurriculumId calculation and
CURRICULUM_MISMATCH check from SessionLifeCycleUsecase.start, leaving curriculum
validation and existing-session resume behavior to
SessionLifecycleService.start. Preserve the existing user and curriculum lookups
and delegate directly with the original request.
In
`@src/main/kotlin/org/prography/samsung/backend/user/dto/response/UserSettingsResponse.kt`:
- Line 4: Remove the redundant UserScheduleResponse import from
UserSettingsResponse, since both classes share the same package; leave all
usages and other imports unchanged.
In
`@src/test/kotlin/org/prography/samsung/backend/session/service/SessionServiceTest.kt`:
- Line 27: Rename the test class `SessionServiceTest` to
`SessionLifecycleServiceTest` so it matches the `SessionLifecycleService` SUT
and its existing display name.
---
Outside diff comments:
In
`@src/main/kotlin/org/prography/samsung/backend/session/service/SessionCompletionService.kt`:
- Around line 21-27: Remove direct UserProfileRepository,
UserCurriculumRepository, and BadgeLevelRepository dependencies from
SessionCompletionService, and delegate profile/curriculum progress updates and
badge selection to the owning domain service(s). Keep CoinLedgerEntryRepository
in the session service and preserve the existing completion flow through
service-level operations.
- Around line 79-88: The session reward logic in complete() is vulnerable to
concurrent duplicate rewards because existsBySessionId() and save() are not
atomic and the flow lacks a transaction boundary. Add a persistence-level
uniqueness/atomic guard for the session reward, make complete() transactional so
profile updates and CoinLedgerEntry persistence commit as one unit, and handle
duplicate attempts without awarding coins twice.
In
`@src/test/kotlin/org/prography/samsung/backend/session/service/SessionServiceTest.kt`:
- Line 126: Extend SessionQueryServiceTest with the pending-reward (“미수령 보상 세션”)
getStatus scenario removed from SessionServiceTest. Recreate the equivalent
setup and assertion for the pending-reward session, ensuring the test verifies
the expected getStatus result.
---
Nitpick comments:
In @.agents/skills/architecture-skill/report-template.md:
- Line 5: Update the fenced code block in the report template’s Markdown content
to include the `markdown` language specifier, preserving the existing template
content unchanged.
In @.agents/skills/architecture-skill/SKILL.md:
- Around line 33-41: Update the fenced code block containing the architecture
path mappings in SKILL.md to specify the text language, changing the opening
fence to use text while preserving its contents.
In
`@src/main/kotlin/org/prography/samsung/backend/gamification/dto/response/RewardResponse.kt`:
- Around line 18-31: Extract the mapping logic from RewardResponse.from into a
dedicated RewardMapper, following the existing HintNoteMapper pattern and
placing it in the appropriate session or gamification service layer. Move the
session, profile, badgeLevelUp, and newBadge mapping there, then remove the DTO
factory and update callers to use the mapper while preserving the current
defaults and response values.
In
`@src/main/kotlin/org/prography/samsung/backend/user/dto/request/UserSettingsRequest.kt`:
- Around line 22-27: The `@Schema` pattern on UserSettingsRequest.time currently
permits every hour, while schedule validation only accepts 15:00 through 20:00.
Update the pattern to document exactly those allowed hourly values, preserving
the nullable field and existing example.
In
`@src/main/kotlin/org/prography/samsung/backend/user/service/UserProfileService.kt`:
- Around line 22-26: Update getUserProfileResponse to obtain the profile through
the existing getUserProfile helper instead of directly calling
userProfileRepository.findById(...).orElseThrow(...). Preserve the current
USER_NOT_FOUND behavior and leave the helper as the single profile-lookup
implementation.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 40b705b0-29be-4294-b2ab-a44cce61c36d
📒 Files selected for processing (76)
.agents/skills/architecture-skill/SKILL.md.agents/skills/architecture-skill/report-template.md.agents/skills/architecture-skill/rules.md.claude/skills/architecture-skill/SKILL.md.claude/skills/architecture-skill/report-template.md.claude/skills/architecture-skill/rules.md.gitignoreAGENTS.mdsrc/main/kotlin/org/prography/samsung/backend/common/dto/SharedDtos.ktsrc/main/kotlin/org/prography/samsung/backend/common/response/DomainErrorCode.ktsrc/main/kotlin/org/prography/samsung/backend/curriculum/dto/response/CurriculumChipResponse.ktsrc/main/kotlin/org/prography/samsung/backend/curriculum/dto/response/CurriculumSummaryResponse.ktsrc/main/kotlin/org/prography/samsung/backend/curriculum/dto/response/HintNoteResponse.ktsrc/main/kotlin/org/prography/samsung/backend/curriculum/dto/response/LessonContent.ktsrc/main/kotlin/org/prography/samsung/backend/curriculum/dto/response/LessonQuestionResponse.ktsrc/main/kotlin/org/prography/samsung/backend/curriculum/dto/response/TodayTopicResponse.ktsrc/main/kotlin/org/prography/samsung/backend/curriculum/service/CurriculumService.ktsrc/main/kotlin/org/prography/samsung/backend/curriculum/service/HintNoteMapper.ktsrc/main/kotlin/org/prography/samsung/backend/gamification/dto/response/LevelResponse.ktsrc/main/kotlin/org/prography/samsung/backend/gamification/dto/response/RewardResponse.ktsrc/main/kotlin/org/prography/samsung/backend/notification/controller/NotificationController.ktsrc/main/kotlin/org/prography/samsung/backend/notification/dto/request/NotificationRegisterRequest.ktsrc/main/kotlin/org/prography/samsung/backend/notification/dto/response/NotificationRegisterResponse.ktsrc/main/kotlin/org/prography/samsung/backend/notification/dto/response/NotificationRescheduleResponse.ktsrc/main/kotlin/org/prography/samsung/backend/notification/service/NotificationService.ktsrc/main/kotlin/org/prography/samsung/backend/session/controller/SessionController.ktsrc/main/kotlin/org/prography/samsung/backend/session/dto/SessionDtos.ktsrc/main/kotlin/org/prography/samsung/backend/session/dto/request/SessionCompleteRequest.ktsrc/main/kotlin/org/prography/samsung/backend/session/dto/request/SessionStartRequest.ktsrc/main/kotlin/org/prography/samsung/backend/session/dto/response/ActiveSessionResponse.ktsrc/main/kotlin/org/prography/samsung/backend/session/dto/response/RewardAckResponse.ktsrc/main/kotlin/org/prography/samsung/backend/session/dto/response/SessionAbortResponse.ktsrc/main/kotlin/org/prography/samsung/backend/session/dto/response/SessionHistoryItemResponse.ktsrc/main/kotlin/org/prography/samsung/backend/session/dto/response/SessionHistoryResponse.ktsrc/main/kotlin/org/prography/samsung/backend/session/dto/response/SessionLessonResponse.ktsrc/main/kotlin/org/prography/samsung/backend/session/dto/response/SessionPhaseResponse.ktsrc/main/kotlin/org/prography/samsung/backend/session/dto/response/SessionStartResponse.ktsrc/main/kotlin/org/prography/samsung/backend/session/dto/response/SessionStatusResponse.ktsrc/main/kotlin/org/prography/samsung/backend/session/dto/response/SessionTodayResponse.ktsrc/main/kotlin/org/prography/samsung/backend/session/service/SessionCompletionService.ktsrc/main/kotlin/org/prography/samsung/backend/session/service/SessionHistoryService.ktsrc/main/kotlin/org/prography/samsung/backend/session/service/SessionLifecycleService.ktsrc/main/kotlin/org/prography/samsung/backend/session/service/SessionQueryService.ktsrc/main/kotlin/org/prography/samsung/backend/session/service/SessionRewardService.ktsrc/main/kotlin/org/prography/samsung/backend/session/service/SessionService.ktsrc/main/kotlin/org/prography/samsung/backend/session/usecase/SessionCompletionUsecase.ktsrc/main/kotlin/org/prography/samsung/backend/session/usecase/SessionHistoryUsecase.ktsrc/main/kotlin/org/prography/samsung/backend/session/usecase/SessionLessonUsecase.ktsrc/main/kotlin/org/prography/samsung/backend/session/usecase/SessionLifeCycleUsecase.ktsrc/main/kotlin/org/prography/samsung/backend/user/controller/OnboardingController.ktsrc/main/kotlin/org/prography/samsung/backend/user/controller/UserController.ktsrc/main/kotlin/org/prography/samsung/backend/user/dto/UserDtos.ktsrc/main/kotlin/org/prography/samsung/backend/user/dto/request/OnboardingRequest.ktsrc/main/kotlin/org/prography/samsung/backend/user/dto/request/UserScheduleRequest.ktsrc/main/kotlin/org/prography/samsung/backend/user/dto/request/UserSettingsRequest.ktsrc/main/kotlin/org/prography/samsung/backend/user/dto/response/OnboardingCompleteResponse.ktsrc/main/kotlin/org/prography/samsung/backend/user/dto/response/OnboardingResponse.ktsrc/main/kotlin/org/prography/samsung/backend/user/dto/response/OnboardingStatusResponse.ktsrc/main/kotlin/org/prography/samsung/backend/user/dto/response/UserHomeResponse.ktsrc/main/kotlin/org/prography/samsung/backend/user/dto/response/UserProfileResponse.ktsrc/main/kotlin/org/prography/samsung/backend/user/dto/response/UserScheduleResponse.ktsrc/main/kotlin/org/prography/samsung/backend/user/dto/response/UserSettingsResponse.ktsrc/main/kotlin/org/prography/samsung/backend/user/service/OnboardingService.ktsrc/main/kotlin/org/prography/samsung/backend/user/service/UserProfileService.ktsrc/main/kotlin/org/prography/samsung/backend/user/service/UserSettingsService.ktsrc/main/kotlin/org/prography/samsung/backend/user/usecase/OnboardingUsecase.ktsrc/main/kotlin/org/prography/samsung/backend/user/usecase/UserHomeUsecase.ktsrc/main/kotlin/org/prography/samsung/backend/user/usecase/UserProfileUsecase.ktsrc/test/kotlin/org/prography/samsung/backend/curriculum/service/CurriculumServiceTest.ktsrc/test/kotlin/org/prography/samsung/backend/notification/service/NotificationServiceTest.ktsrc/test/kotlin/org/prography/samsung/backend/session/service/SessionServiceTest.ktsrc/test/kotlin/org/prography/samsung/backend/user/UserApiIntegrationTest.ktsrc/test/kotlin/org/prography/samsung/backend/user/service/OnboardingServiceTest.ktsrc/test/kotlin/org/prography/samsung/backend/user/service/UserProfileServiceTest.ktsrc/test/kotlin/org/prography/samsung/backend/user/service/UserSettingsServiceTest.ktsrc/test/kotlin/org/prography/samsung/backend/user/usecase/UserHomeUsecaseTest.kt
💤 Files with no reviewable changes (4)
- src/main/kotlin/org/prography/samsung/backend/user/dto/UserDtos.kt
- src/main/kotlin/org/prography/samsung/backend/session/dto/SessionDtos.kt
- src/main/kotlin/org/prography/samsung/backend/common/dto/SharedDtos.kt
- src/main/kotlin/org/prography/samsung/backend/session/service/SessionService.kt
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/main/kotlin/org/prography/samsung/backend/session/service/SessionCompletionService.kt (2)
21-27: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftMove cross-domain reward updates behind domain services
UserProfileRepository,UserCurriculumRepository, andBadgeLevelRepositorylive outside the session domain, soSessionCompletionServiceis now handling user/gamification mutations directly. KeepCoinLedgerEntryRepositoryhere, but move the profile/curriculum progress update and badge selection behind the owning service(s) instead of injecting those repositories into session.🤖 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 `@src/main/kotlin/org/prography/samsung/backend/session/service/SessionCompletionService.kt` around lines 21 - 27, Remove direct UserProfileRepository, UserCurriculumRepository, and BadgeLevelRepository dependencies from SessionCompletionService, and delegate profile/curriculum progress updates and badge selection to the owning domain service(s). Keep CoinLedgerEntryRepository in the session service and preserve the existing completion flow through service-level operations.
79-88: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftMake the session reward path atomic
existsBySessionId()is still a check-then-act gap here, and the completion flow has no transaction boundary. Two concurrentcomplete()calls can both apply the profile reward before any later session conflict, so one session can still mint coins twice. Add an atomic guard at the persistence layer and commit the whole flow as one unit.🤖 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 `@src/main/kotlin/org/prography/samsung/backend/session/service/SessionCompletionService.kt` around lines 79 - 88, The session reward logic in complete() is vulnerable to concurrent duplicate rewards because existsBySessionId() and save() are not atomic and the flow lacks a transaction boundary. Add a persistence-level uniqueness/atomic guard for the session reward, make complete() transactional so profile updates and CoinLedgerEntry persistence commit as one unit, and handle duplicate attempts without awarding coins twice.src/test/kotlin/org/prography/samsung/backend/session/service/SessionServiceTest.kt (1)
126-126: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winAdd the pending-reward
getStatuscase toSessionQueryServiceTest.
The미수령 보상 세션coverage was removed fromSessionServiceTest, and there’s no matching test inSessionQueryServiceTest. Add the equivalent assertion there to keepgetStatuscovered.🤖 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 `@src/test/kotlin/org/prography/samsung/backend/session/service/SessionServiceTest.kt` at line 126, Extend SessionQueryServiceTest with the pending-reward (“미수령 보상 세션”) getStatus scenario removed from SessionServiceTest. Recreate the equivalent setup and assertion for the pending-reward session, ensuring the test verifies the expected getStatus result.
🧹 Nitpick comments (5)
.agents/skills/architecture-skill/SKILL.md (1)
33-41: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider specifying a language for the fenced code block.
The code block starting at line 33 has no language specifier, triggering markdownlint MD040. Adding
textas the language keeps it informative for syntax highlighters.♻️ Optional fix
`
-+text
.../{domain}/controller/X.kt → (domain, controller)</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In @.agents/skills/architecture-skill/SKILL.md around lines 33 - 41, Update the
fenced code block containing the architecture path mappings in SKILL.md to
specify the text language, changing the opening fence to use text while
preserving its contents.</details> <!-- cr-comment:v1:d72c9eca2f05b6e52c5900c4 --> _Source: Linters/SAST tools_ </blockquote></details> <details> <summary>.agents/skills/architecture-skill/report-template.md (1)</summary><blockquote> `5-5`: _📐 Maintainability & Code Quality_ | _🔵 Trivial_ | _💤 Low value_ **Consider specifying a language for the fenced code block.** The code block starting at line 5 has no language specifier (markdownlint MD040). Since it contains Markdown template content, `markdown` would be the appropriate language. <details> <summary>♻️ Optional fix</summary> ```diff -``` +```markdown ## 🏛 아키텍처 검증 리포트🤖 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 @.agents/skills/architecture-skill/report-template.md at line 5, Update the fenced code block in the report template’s Markdown content to include the `markdown` language specifier, preserving the existing template content unchanged.Source: Linters/SAST tools
src/main/kotlin/org/prography/samsung/backend/gamification/dto/response/RewardResponse.kt (1)
18-31: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting the
from()mapping logic to a dedicated mapper.
RewardResponse.from()imports entities from three domains (session,user,gamification), coupling a gamification DTO to session and user entities. The project already uses a separateHintNoteMapperpattern in the curriculum domain. Moving this mapping to a dedicated mapper (e.g.,RewardMapperin the session or gamification service layer) would keep the DTO pure and centralize cross-domain coordination where it's architecturally appropriate.This is not blocking — the factory pattern is idiomatic Kotlin and the logic itself is correct.
🤖 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 `@src/main/kotlin/org/prography/samsung/backend/gamification/dto/response/RewardResponse.kt` around lines 18 - 31, Extract the mapping logic from RewardResponse.from into a dedicated RewardMapper, following the existing HintNoteMapper pattern and placing it in the appropriate session or gamification service layer. Move the session, profile, badgeLevelUp, and newBadge mapping there, then remove the DTO factory and update callers to use the mapper while preserving the current defaults and response values.src/main/kotlin/org/prography/samsung/backend/user/dto/request/UserSettingsRequest.kt (1)
22-27: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueAlign the
@Schematime pattern with the actual schedule rule. The current regex documents00:00–23:00, but the schedule validator only accepts15:00–20:00on the hour.🤖 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 `@src/main/kotlin/org/prography/samsung/backend/user/dto/request/UserSettingsRequest.kt` around lines 22 - 27, The `@Schema` pattern on UserSettingsRequest.time currently permits every hour, while schedule validation only accepts 15:00 through 20:00. Update the pattern to document exactly those allowed hourly values, preserving the nullable field and existing example.src/main/kotlin/org/prography/samsung/backend/user/service/UserProfileService.kt (1)
22-26: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsolidate duplicated profile lookup.
getUserProfileResponsedirectly callsuserProfileRepository.findById(userId).orElseThrow { ... }(lines 23-26), duplicating the exact logic ingetUserProfile(lines 43-45). Use the helper to keep error handling consistent and reduce duplication.♻️ Proposed refactor
fun getUserProfileResponse(userId: Long): UserProfileResponse { val profile = - userProfileRepository.findById(userId).orElseThrow { - CustomException(DomainErrorCode.USER_NOT_FOUND) - } + getUserProfile(userId) val userCurriculum = getUserCurriculum(userId)Also applies to: 43-45
🤖 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 `@src/main/kotlin/org/prography/samsung/backend/user/service/UserProfileService.kt` around lines 22 - 26, Update getUserProfileResponse to obtain the profile through the existing getUserProfile helper instead of directly calling userProfileRepository.findById(...).orElseThrow(...). Preserve the current USER_NOT_FOUND behavior and leave the helper as the single profile-lookup implementation.
🤖 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
`@src/main/kotlin/org/prography/samsung/backend/curriculum/dto/response/LessonContent.kt`:
- Around line 1-3: Rename the outbound DTO class LessonContent to
LessonContentResponse, preserving its constructor properties and data-class
behavior, and update all references including
CurriculumService.getLessonContent() and its callers to use the new symbol.
In
`@src/main/kotlin/org/prography/samsung/backend/curriculum/service/CurriculumService.kt`:
- Around line 64-79: Update the null-handling in
CurriculumService.getLessonContent to throw a dedicated question-not-found error
instead of DomainErrorCode.LESSON_TOPIC_NOT_FOUND. Add a unique
LESSON_QUESTION_NOT_FOUND entry to DomainErrorCode, using the project’s existing
error-code conventions, and reference it when
lessonQuestionRepository.findByLessonTopicIdAndPhase returns null; preserve the
existing hint-note error handling.
In
`@src/main/kotlin/org/prography/samsung/backend/session/service/SessionCompletionService.kt`:
- Around line 34-36: Update the completed-session branch in
SessionCompletionService to replace the no-argument orElseThrow on
userProfileRepository.findById(userId) with the same
CustomException(DomainErrorCode.USER_NOT_FOUND) supplier used in the normal
path, preserving the RewardResponse behavior when the profile exists.
In
`@src/main/kotlin/org/prography/samsung/backend/session/service/SessionLifecycleService.kt`:
- Around line 33-84: Enforce at the persistence layer that each user can have at
most one TutoringSession with SessionStatus.STARTED, using a database unique
constraint or equivalent atomic locking/conditional insert. Update the
repository/entity configuration around start and findByUserIdAndStatus so
concurrent SessionLifecycleUsecase.start requests cannot create duplicate active
sessions, while preserving the existing resume behavior.
In
`@src/main/kotlin/org/prography/samsung/backend/session/usecase/SessionLifeCycleUsecase.kt`:
- Around line 14-15: Rename SessionLifeCycleUsecase to SessionLifecycleUsecase,
including the Kotlin file, class declaration, and all references. Update the
import and field usage in SessionController to use the consistently spelled
symbol.
- Around line 20-27: Remove the local targetCurriculumId calculation and
CURRICULUM_MISMATCH check from SessionLifeCycleUsecase.start, leaving curriculum
validation and existing-session resume behavior to
SessionLifecycleService.start. Preserve the existing user and curriculum lookups
and delegate directly with the original request.
In
`@src/main/kotlin/org/prography/samsung/backend/user/dto/response/UserSettingsResponse.kt`:
- Line 4: Remove the redundant UserScheduleResponse import from
UserSettingsResponse, since both classes share the same package; leave all
usages and other imports unchanged.
In
`@src/test/kotlin/org/prography/samsung/backend/session/service/SessionServiceTest.kt`:
- Line 27: Rename the test class `SessionServiceTest` to
`SessionLifecycleServiceTest` so it matches the `SessionLifecycleService` SUT
and its existing display name.
---
Outside diff comments:
In
`@src/main/kotlin/org/prography/samsung/backend/session/service/SessionCompletionService.kt`:
- Around line 21-27: Remove direct UserProfileRepository,
UserCurriculumRepository, and BadgeLevelRepository dependencies from
SessionCompletionService, and delegate profile/curriculum progress updates and
badge selection to the owning domain service(s). Keep CoinLedgerEntryRepository
in the session service and preserve the existing completion flow through
service-level operations.
- Around line 79-88: The session reward logic in complete() is vulnerable to
concurrent duplicate rewards because existsBySessionId() and save() are not
atomic and the flow lacks a transaction boundary. Add a persistence-level
uniqueness/atomic guard for the session reward, make complete() transactional so
profile updates and CoinLedgerEntry persistence commit as one unit, and handle
duplicate attempts without awarding coins twice.
In
`@src/test/kotlin/org/prography/samsung/backend/session/service/SessionServiceTest.kt`:
- Line 126: Extend SessionQueryServiceTest with the pending-reward (“미수령 보상 세션”)
getStatus scenario removed from SessionServiceTest. Recreate the equivalent
setup and assertion for the pending-reward session, ensuring the test verifies
the expected getStatus result.
---
Nitpick comments:
In @.agents/skills/architecture-skill/report-template.md:
- Line 5: Update the fenced code block in the report template’s Markdown content
to include the `markdown` language specifier, preserving the existing template
content unchanged.
In @.agents/skills/architecture-skill/SKILL.md:
- Around line 33-41: Update the fenced code block containing the architecture
path mappings in SKILL.md to specify the text language, changing the opening
fence to use text while preserving its contents.
In
`@src/main/kotlin/org/prography/samsung/backend/gamification/dto/response/RewardResponse.kt`:
- Around line 18-31: Extract the mapping logic from RewardResponse.from into a
dedicated RewardMapper, following the existing HintNoteMapper pattern and
placing it in the appropriate session or gamification service layer. Move the
session, profile, badgeLevelUp, and newBadge mapping there, then remove the DTO
factory and update callers to use the mapper while preserving the current
defaults and response values.
In
`@src/main/kotlin/org/prography/samsung/backend/user/dto/request/UserSettingsRequest.kt`:
- Around line 22-27: The `@Schema` pattern on UserSettingsRequest.time currently
permits every hour, while schedule validation only accepts 15:00 through 20:00.
Update the pattern to document exactly those allowed hourly values, preserving
the nullable field and existing example.
In
`@src/main/kotlin/org/prography/samsung/backend/user/service/UserProfileService.kt`:
- Around line 22-26: Update getUserProfileResponse to obtain the profile through
the existing getUserProfile helper instead of directly calling
userProfileRepository.findById(...).orElseThrow(...). Preserve the current
USER_NOT_FOUND behavior and leave the helper as the single profile-lookup
implementation.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 40b705b0-29be-4294-b2ab-a44cce61c36d
📒 Files selected for processing (76)
.agents/skills/architecture-skill/SKILL.md.agents/skills/architecture-skill/report-template.md.agents/skills/architecture-skill/rules.md.claude/skills/architecture-skill/SKILL.md.claude/skills/architecture-skill/report-template.md.claude/skills/architecture-skill/rules.md.gitignoreAGENTS.mdsrc/main/kotlin/org/prography/samsung/backend/common/dto/SharedDtos.ktsrc/main/kotlin/org/prography/samsung/backend/common/response/DomainErrorCode.ktsrc/main/kotlin/org/prography/samsung/backend/curriculum/dto/response/CurriculumChipResponse.ktsrc/main/kotlin/org/prography/samsung/backend/curriculum/dto/response/CurriculumSummaryResponse.ktsrc/main/kotlin/org/prography/samsung/backend/curriculum/dto/response/HintNoteResponse.ktsrc/main/kotlin/org/prography/samsung/backend/curriculum/dto/response/LessonContent.ktsrc/main/kotlin/org/prography/samsung/backend/curriculum/dto/response/LessonQuestionResponse.ktsrc/main/kotlin/org/prography/samsung/backend/curriculum/dto/response/TodayTopicResponse.ktsrc/main/kotlin/org/prography/samsung/backend/curriculum/service/CurriculumService.ktsrc/main/kotlin/org/prography/samsung/backend/curriculum/service/HintNoteMapper.ktsrc/main/kotlin/org/prography/samsung/backend/gamification/dto/response/LevelResponse.ktsrc/main/kotlin/org/prography/samsung/backend/gamification/dto/response/RewardResponse.ktsrc/main/kotlin/org/prography/samsung/backend/notification/controller/NotificationController.ktsrc/main/kotlin/org/prography/samsung/backend/notification/dto/request/NotificationRegisterRequest.ktsrc/main/kotlin/org/prography/samsung/backend/notification/dto/response/NotificationRegisterResponse.ktsrc/main/kotlin/org/prography/samsung/backend/notification/dto/response/NotificationRescheduleResponse.ktsrc/main/kotlin/org/prography/samsung/backend/notification/service/NotificationService.ktsrc/main/kotlin/org/prography/samsung/backend/session/controller/SessionController.ktsrc/main/kotlin/org/prography/samsung/backend/session/dto/SessionDtos.ktsrc/main/kotlin/org/prography/samsung/backend/session/dto/request/SessionCompleteRequest.ktsrc/main/kotlin/org/prography/samsung/backend/session/dto/request/SessionStartRequest.ktsrc/main/kotlin/org/prography/samsung/backend/session/dto/response/ActiveSessionResponse.ktsrc/main/kotlin/org/prography/samsung/backend/session/dto/response/RewardAckResponse.ktsrc/main/kotlin/org/prography/samsung/backend/session/dto/response/SessionAbortResponse.ktsrc/main/kotlin/org/prography/samsung/backend/session/dto/response/SessionHistoryItemResponse.ktsrc/main/kotlin/org/prography/samsung/backend/session/dto/response/SessionHistoryResponse.ktsrc/main/kotlin/org/prography/samsung/backend/session/dto/response/SessionLessonResponse.ktsrc/main/kotlin/org/prography/samsung/backend/session/dto/response/SessionPhaseResponse.ktsrc/main/kotlin/org/prography/samsung/backend/session/dto/response/SessionStartResponse.ktsrc/main/kotlin/org/prography/samsung/backend/session/dto/response/SessionStatusResponse.ktsrc/main/kotlin/org/prography/samsung/backend/session/dto/response/SessionTodayResponse.ktsrc/main/kotlin/org/prography/samsung/backend/session/service/SessionCompletionService.ktsrc/main/kotlin/org/prography/samsung/backend/session/service/SessionHistoryService.ktsrc/main/kotlin/org/prography/samsung/backend/session/service/SessionLifecycleService.ktsrc/main/kotlin/org/prography/samsung/backend/session/service/SessionQueryService.ktsrc/main/kotlin/org/prography/samsung/backend/session/service/SessionRewardService.ktsrc/main/kotlin/org/prography/samsung/backend/session/service/SessionService.ktsrc/main/kotlin/org/prography/samsung/backend/session/usecase/SessionCompletionUsecase.ktsrc/main/kotlin/org/prography/samsung/backend/session/usecase/SessionHistoryUsecase.ktsrc/main/kotlin/org/prography/samsung/backend/session/usecase/SessionLessonUsecase.ktsrc/main/kotlin/org/prography/samsung/backend/session/usecase/SessionLifeCycleUsecase.ktsrc/main/kotlin/org/prography/samsung/backend/user/controller/OnboardingController.ktsrc/main/kotlin/org/prography/samsung/backend/user/controller/UserController.ktsrc/main/kotlin/org/prography/samsung/backend/user/dto/UserDtos.ktsrc/main/kotlin/org/prography/samsung/backend/user/dto/request/OnboardingRequest.ktsrc/main/kotlin/org/prography/samsung/backend/user/dto/request/UserScheduleRequest.ktsrc/main/kotlin/org/prography/samsung/backend/user/dto/request/UserSettingsRequest.ktsrc/main/kotlin/org/prography/samsung/backend/user/dto/response/OnboardingCompleteResponse.ktsrc/main/kotlin/org/prography/samsung/backend/user/dto/response/OnboardingResponse.ktsrc/main/kotlin/org/prography/samsung/backend/user/dto/response/OnboardingStatusResponse.ktsrc/main/kotlin/org/prography/samsung/backend/user/dto/response/UserHomeResponse.ktsrc/main/kotlin/org/prography/samsung/backend/user/dto/response/UserProfileResponse.ktsrc/main/kotlin/org/prography/samsung/backend/user/dto/response/UserScheduleResponse.ktsrc/main/kotlin/org/prography/samsung/backend/user/dto/response/UserSettingsResponse.ktsrc/main/kotlin/org/prography/samsung/backend/user/service/OnboardingService.ktsrc/main/kotlin/org/prography/samsung/backend/user/service/UserProfileService.ktsrc/main/kotlin/org/prography/samsung/backend/user/service/UserSettingsService.ktsrc/main/kotlin/org/prography/samsung/backend/user/usecase/OnboardingUsecase.ktsrc/main/kotlin/org/prography/samsung/backend/user/usecase/UserHomeUsecase.ktsrc/main/kotlin/org/prography/samsung/backend/user/usecase/UserProfileUsecase.ktsrc/test/kotlin/org/prography/samsung/backend/curriculum/service/CurriculumServiceTest.ktsrc/test/kotlin/org/prography/samsung/backend/notification/service/NotificationServiceTest.ktsrc/test/kotlin/org/prography/samsung/backend/session/service/SessionServiceTest.ktsrc/test/kotlin/org/prography/samsung/backend/user/UserApiIntegrationTest.ktsrc/test/kotlin/org/prography/samsung/backend/user/service/OnboardingServiceTest.ktsrc/test/kotlin/org/prography/samsung/backend/user/service/UserProfileServiceTest.ktsrc/test/kotlin/org/prography/samsung/backend/user/service/UserSettingsServiceTest.ktsrc/test/kotlin/org/prography/samsung/backend/user/usecase/UserHomeUsecaseTest.kt
💤 Files with no reviewable changes (4)
- src/main/kotlin/org/prography/samsung/backend/user/dto/UserDtos.kt
- src/main/kotlin/org/prography/samsung/backend/session/dto/SessionDtos.kt
- src/main/kotlin/org/prography/samsung/backend/common/dto/SharedDtos.kt
- src/main/kotlin/org/prography/samsung/backend/session/service/SessionService.kt
🛑 Comments failed to post (8)
src/main/kotlin/org/prography/samsung/backend/curriculum/dto/response/LessonContent.kt (1)
1-3: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Rename
LessonContenttoLessonContentResponseto follow the DTO suffix convention.As per coding guidelines, outbound DTOs must use the
*Responsesuffix.LessonContentis in thedto/responsepackage and returned byCurriculumService.getLessonContent(), making it an outbound DTO. All sibling DTOs in the same package already follow this convention.♻️ Proposed rename
-data class LessonContent(val question: LessonQuestionResponse, val hintNote: HintNoteResponse) +data class LessonContentResponse(val question: LessonQuestionResponse, val hintNote: HintNoteResponse)📝 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.package org.prography.samsung.backend.curriculum.dto.response data class LessonContentResponse(val question: LessonQuestionResponse, val hintNote: HintNoteResponse)🤖 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 `@src/main/kotlin/org/prography/samsung/backend/curriculum/dto/response/LessonContent.kt` around lines 1 - 3, Rename the outbound DTO class LessonContent to LessonContentResponse, preserving its constructor properties and data-class behavior, and update all references including CurriculumService.getLessonContent() and its callers to use the new symbol.Source: Coding guidelines
src/main/kotlin/org/prography/samsung/backend/curriculum/service/CurriculumService.kt (1)
64-79: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Wrong error code thrown when lesson question is not found.
Line 67 throws
DomainErrorCode.LESSON_TOPIC_NOT_FOUND("학습 주제를 찾을 수 없습니다") whenlessonQuestionRepository.findByLessonTopicIdAndPhasereturns null. The question is missing, not the topic. This produces a misleading 404 error message for API consumers who know the topic exists.Consider adding a
LESSON_QUESTION_NOT_FOUNDentry toDomainErrorCode(e.g., code40460) and using it here instead. As per coding guidelines, new domain error codes should be added toDomainErrorCodewith a unique code.🐛 Proposed fix
In
DomainErrorCode.kt:HINT_NOTE_NOT_FOUND(HttpStatus.NOT_FOUND, 40450, "힌트 노트를 찾을 수 없습니다."), + LESSON_QUESTION_NOT_FOUND(HttpStatus.NOT_FOUND, 40460, "학습 문제를 찾을 수 없습니다."),In
CurriculumService.kt:- val question = lessonQuestionRepository.findByLessonTopicIdAndPhase(lessonTopicId, phase) - ?: throw CustomException(DomainErrorCode.LESSON_TOPIC_NOT_FOUND) + val question = lessonQuestionRepository.findByLessonTopicIdAndPhase(lessonTopicId, phase) + ?: throw CustomException(DomainErrorCode.LESSON_QUESTION_NOT_FOUND)🤖 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 `@src/main/kotlin/org/prography/samsung/backend/curriculum/service/CurriculumService.kt` around lines 64 - 79, Update the null-handling in CurriculumService.getLessonContent to throw a dedicated question-not-found error instead of DomainErrorCode.LESSON_TOPIC_NOT_FOUND. Add a unique LESSON_QUESTION_NOT_FOUND entry to DomainErrorCode, using the project’s existing error-code conventions, and reference it when lessonQuestionRepository.findByLessonTopicIdAndPhase returns null; preserve the existing hint-note error handling.Source: Coding guidelines
src/main/kotlin/org/prography/samsung/backend/session/service/SessionCompletionService.kt (1)
34-36: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Raw
NoSuchElementExceptioninstead ofCustomException.
userProfileRepository.findById(userId).orElseThrow()(no-arg) throwsNoSuchElementExceptionwhen empty, bypassing the domain error contract used everywhere else in this same method (Lines 43-45 use.orElseThrow { CustomException(DomainErrorCode.USER_NOT_FOUND) }). If the profile is missing on this already-completed-session path, the caller gets an unhandled raw runtime exception instead of a properCustomException(DomainErrorCode...).🐛 Proposed fix
if (session.status == SessionStatus.COMPLETED) { - return RewardResponse.from(session, userProfileRepository.findById(userId).orElseThrow()) + val profile = + userProfileRepository.findById(userId).orElseThrow { + CustomException(DomainErrorCode.USER_NOT_FOUND) + } + return RewardResponse.from(session, profile) }As per path instructions,
src/main/kotlin/org/prography/samsung/backend/**/service/**/*.kt: "In services, throwCustomExceptionwithDomainErrorCode; never throw rawRuntimeExceptionorErrorBaseCodedirectly."📝 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.if (session.status == SessionStatus.COMPLETED) { val profile = userProfileRepository.findById(userId).orElseThrow { CustomException(DomainErrorCode.USER_NOT_FOUND) } return RewardResponse.from(session, profile) }🤖 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 `@src/main/kotlin/org/prography/samsung/backend/session/service/SessionCompletionService.kt` around lines 34 - 36, Update the completed-session branch in SessionCompletionService to replace the no-argument orElseThrow on userProfileRepository.findById(userId) with the same CustomException(DomainErrorCode.USER_NOT_FOUND) supplier used in the normal path, preserving the RewardResponse behavior when the profile exists.Source: Path instructions
src/main/kotlin/org/prography/samsung/backend/session/service/SessionLifecycleService.kt (1)
33-84: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash fd -a 'SessionLifeCycleUsecase.kt' --exec cat {} rg -n 'Version|uniqueConstraints|Index|Table' src/main/kotlin/org/prography/samsung/backend/session/entity/TutoringSession.ktRepository: prography/11th-3team-BE
Length of output: 2074
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect the tutoring session entity and nearby annotations. entity=$(fd -a 'TutoringSession.kt' src/main/kotlin/org/prography/samsung/backend/session/entity | head -n 1) echo "ENTITY=$entity" wc -l "$entity" sed -n '1,220p' "$entity" # Search for constraints or schema definitions related to tutoring_sessions / active sessions. rg -n 'tutoring_sessions|uniqueConstraints|`@Index`|`@Table`|active session|STARTED|SessionStatus.STARTED|user_id' \ src/main/kotlin src/main/resources db . 2>/dev/null | sed -n '1,220p'Repository: prography/11th-3team-BE
Length of output: 29400
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect the session creation table definition for any uniqueness guard on active sessions. sed -n '1,80p' src/main/resources/db/migration/V4__create_session_tables.sql # Check whether any migration adds a uniqueness constraint tied to tutoring_sessions.user_id/status. rg -n 'UNIQUE|unique|idx_tutoring_sessions_user_status|tutoring_sessions \(user_id|user_id.*status|status.*user_id' \ src/main/resources/db/migration src/main/kotlin/org/prography/samsung/backend/session \ | sed -n '1,160p'Repository: prography/11th-3team-BE
Length of output: 3233
🏁 Script executed:
#!/bin/bash set -euo pipefail # Check whether the active-session lookup uses any pessimistic/optimistic locking. rg -n '`@Lock`|PESSIMISTIC|OPTIMISTIC|findByUserIdAndStatus|tutoringSessionRepository' \ src/main/kotlin/org/prography/samsung/backend/session src/main/kotlin/org/prography/samsung/backend/user \ | sed -n '1,200p' # Read the session repository interface. repo=$(fd -a 'TutoringSessionRepository.kt' src/main/kotlin | head -n 1) echo "REPO=$repo" wc -l "$repo" sed -n '1,220p' "$repo"Repository: prography/11th-3team-BE
Length of output: 7154
Prevent duplicate STARTED sessions at the database layer.
SessionLifeCycleUsecase.start()is transactional, but thefindByUserIdAndStatus(...)check is still race-prone: two concurrent requests can both miss the existing row and insert separateSTARTEDsessions. Add a unique constraint or equivalent lock/conditional insert to enforce one active session per user.🤖 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 `@src/main/kotlin/org/prography/samsung/backend/session/service/SessionLifecycleService.kt` around lines 33 - 84, Enforce at the persistence layer that each user can have at most one TutoringSession with SessionStatus.STARTED, using a database unique constraint or equivalent atomic locking/conditional insert. Update the repository/entity configuration around start and findByUserIdAndStatus so concurrent SessionLifecycleUsecase.start requests cannot create duplicate active sessions, while preserving the existing resume behavior.src/main/kotlin/org/prography/samsung/backend/session/usecase/SessionLifeCycleUsecase.kt (2)
14-15: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Align class naming:
LifeCycle→Lifecyclefor consistency withSessionLifecycleService.The service uses
SessionLifecycleService(lowercase 'c') but the usecase usesSessionLifeCycleUsecase(uppercase 'C'). "Lifecycle" is the standard spelling and matches the service naming.♻️ Proposed fix
-class SessionLifeCycleUsecase( +class SessionLifecycleUsecase( private val sessionLifecycleService: SessionLifecycleService, private val userProfileService: UserProfileService, )Also rename the file to
SessionLifecycleUsecase.ktand update the import/field inSessionController.kt.🤖 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 `@src/main/kotlin/org/prography/samsung/backend/session/usecase/SessionLifeCycleUsecase.kt` around lines 14 - 15, Rename SessionLifeCycleUsecase to SessionLifecycleUsecase, including the Kotlin file, class declaration, and all references. Update the import and field usage in SessionController to use the consistently spelled symbol.
20-27: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Remove redundant curriculum mismatch check — it's incomplete and duplicates the service's validation.
The usecase checks only
request?.curriculumIdagainstuserCurriculum.curriculum.id, butSessionLifecycleService.startalready performs a more thorough check that also coversselectedTopic?.curriculum?.id(i.e.,lessonTopicId-based mismatch). The usecase's check is a strict subset and violates DRY.Additionally, this introduces a behavioral change: the service returns an existing STARTED session early (resumed=true) before its mismatch check, but the usecase's check runs before delegation. So a resume request with a mismatched
curriculumIdwould now throwCURRICULUM_MISMATCHinstead of returning the existing session.♻️ Proposed fix: remove the redundant check and let the service validate
`@Transactional` fun start(userId: Long, request: SessionStartRequest?): SessionStartResponse { val user = userProfileService.getUser(userId) val userCurriculum = userProfileService.getUserCurriculum(userId) - val targetCurriculumId = request?.curriculumId ?: userCurriculum.curriculum.id - if (userCurriculum.curriculum.id != targetCurriculumId) { - throw CustomException(DomainErrorCode.CURRICULUM_MISMATCH) - } return sessionLifecycleService.start(user, userCurriculum, request) }📝 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 start(userId: Long, request: SessionStartRequest?): SessionStartResponse { val user = userProfileService.getUser(userId) val userCurriculum = userProfileService.getUserCurriculum(userId) return sessionLifecycleService.start(user, userCurriculum, request)🤖 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 `@src/main/kotlin/org/prography/samsung/backend/session/usecase/SessionLifeCycleUsecase.kt` around lines 20 - 27, Remove the local targetCurriculumId calculation and CURRICULUM_MISMATCH check from SessionLifeCycleUsecase.start, leaving curriculum validation and existing-session resume behavior to SessionLifecycleService.start. Preserve the existing user and curriculum lookups and delegate directly with the original request.src/main/kotlin/org/prography/samsung/backend/user/dto/response/UserSettingsResponse.kt (1)
4-4: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove redundant same-package import.
UserScheduleResponseis in the same package (org.prography.samsung.backend.user.dto.response) as this file, so the import is unnecessary. ktlint'sno-unused-importsrule will flag this, causing./gradlew ktlintCheckto fail. As per coding guidelines, ktlint must be clean before committing.♻️ Proposed fix
import org.prography.samsung.backend.curriculum.dto.response.CurriculumChipResponse -import org.prography.samsung.backend.user.dto.response.UserScheduleResponse data class UserSettingsResponse(val curriculum: CurriculumChipResponse, val schedule: UserScheduleResponse)📝 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.🤖 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 `@src/main/kotlin/org/prography/samsung/backend/user/dto/response/UserSettingsResponse.kt` at line 4, Remove the redundant UserScheduleResponse import from UserSettingsResponse, since both classes share the same package; leave all usages and other imports unchanged.Source: Coding guidelines
src/test/kotlin/org/prography/samsung/backend/session/service/SessionServiceTest.kt (1)
27-27: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Rename test class to match the SUT.
The class is
SessionServiceTestbut the SUT isSessionLifecycleServiceand the display name says "SessionLifecycleService 단위 테스트". Rename toSessionLifecycleServiceTestfor clarity.♻️ Proposed fix
-@DisplayName("SessionLifecycleService 단위 테스트") -class SessionServiceTest { +@DisplayName("SessionLifecycleService 단위 테스트") +class SessionLifecycleServiceTest {📝 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.class SessionLifecycleServiceTest {🤖 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 `@src/test/kotlin/org/prography/samsung/backend/session/service/SessionServiceTest.kt` at line 27, Rename the test class `SessionServiceTest` to `SessionLifecycleServiceTest` so it matches the `SessionLifecycleService` SUT and its existing display name.
1 participant
PR
PR 요약
이 프로젝트는
Controller → Usecase → Service → Repository계층 방향과 "트랜잭션 경계는 Usecase가 소유"라는 규칙, 그리고 도메인별 DTO·에러 컨벤션을 따르고 있습니다. 하지만 이 규칙들이 이후 PR에서도 지켜지는지 매번 사람이 눈으로 확인하기는 번거롭습니다. 이 PR은 변경분(PR diff)이 위 아키텍처 규칙을 준수하는지 정적 분석으로 검증하는 architecture-skill을 추가합니다.변경된 점
1. architecture-skill 추가 (
.claude/skills/architecture-skill,.agents/skills/architecture-skill)git diff main...HEAD로 변경된*.kt파일을 수집해, import 블록 파싱과 grep으로 아래 규칙군을 정적 검증하는 스킬을 추가했습니다.*Request/*ResponseDTO 위치 검증@Entity의 BaseEntity 상속, service의 JPA 인프라 주입 금지 등@Transactional잔존 금지, usecase 진입 메서드의 트랜잭션 경계 소유@Transactional제거 등)은 일괄 자동 수정을 제안합니다. Claude(.claude/)와 Codex(.agents/) 양쪽에 동일하게 배치했습니다.2. 스킬 지원을 위한 설정 정리 (
AGENTS.md,.gitignore)AGENTS.md에 스킬 관련 안내를 추가하고,.gitignore를 함께 정리했습니다.Summary by CodeRabbit
New Features
Refactor
Documentation