refactor: remove redundant jMolecules comments and improve ID extraction #293

coderabbitai · 2025-09-30T15:33:23Z

🛠️ Refactor suggestion | 🟠 Major

Add @ConsistentCopyVisibility annotation for consistency.

Similar to AspectDefinition.kt (line 23), this data class uses a private backing field (_deviceId) for identity but lacks the @ConsistentCopyVisibility annotation. Without it, the generated copy() method may expose the private constructor parameter, potentially allowing external code to modify the aggregate's identity.

Apply this diff:

+@ConsistentCopyVisibility data class SyncState( private val _deviceId: DeviceId,

📝 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.

Suggested change

data class SyncState(

val deviceId: DeviceId,

private val _deviceId: DeviceId,

val lastSyncAt: Instant?,

val remoteVectorClock: VectorClock,

val lastSuccessfulPush: Instant?,

val lastSuccessfulPull: Instant?,

val syncStatus: SyncStatus,

val pendingChanges: Int = 0,

) {

) : AggregateRoot<SyncState, DeviceId> {

@ConsistentCopyVisibility

data class SyncState(

private val _deviceId: DeviceId,

val lastSyncAt: Instant?,

val remoteVectorClock: VectorClock,

val lastSuccessfulPush: Instant?,

val lastSuccessfulPull: Instant?,

val syncStatus: SyncStatus,

val pendingChanges: Int = 0,

) : AggregateRoot<SyncState, DeviceId> {

// …

}

🤖 Prompt for AI Agents

In contexts/device-synchronization/domain/src/main/kotlin/io/github/kamiazya/scopes/devicesync/domain/entity/SyncState.kt around lines 17 to 25, the data class uses a private backing field `_deviceId` for identity but is missing the @ConsistentCopyVisibility annotation; add the @ConsistentCopyVisibility annotation immediately above the data class declaration (and import it if needed) so the generated copy() keeps the same visibility for the private constructor parameter and prevents external code from altering the aggregate identity.

gemini-code-assist · 2025-09-30T15:30:31Z

The KDoc for the overridden getId() method is empty. While it's an override, providing clear documentation improves code clarity and maintainability, especially for a core concept like an aggregate's ID. Please add a KDoc explaining what this ID represents for a SyncState.

Suggested change

/**

*/

override fun getId(): DeviceId = _deviceId

/**

* The unique identifier for this sync state, which corresponds to the remote device's ID.

*/

override fun getId(): DeviceId = _deviceId

-Original file line number
+Diff line change
@@ Expand Up / @@ -2,4 +2,4 @@ @@
     "scopes": patch
     ---
-    fix: Resolved a release workflow failure by correcting heredoc syntax in `release.yml` to prevent improper variable expansion.
+    fix: Resolved a release workflow failure by correcting heredoc syntax in `release.yml` to prevent improper variable expansion.

-Original file line number
+Diff line change
@@ Expand Up @@
     - 1 unified offline package (~260MB) for enterprise/multi-platform deployments
     - SLSA provenance for supply chain security
-    This provides 92% reduction in download size for most users while maintaining all existing security features (SLSA Level 3, SHA256 verification) and preparing for future daemon binary distribution.
+    This provides 92% reduction in download size for most users while maintaining all existing security features (SLSA Level 3, SHA256 verification) and preparing for future daemon binary distribution.

-Original file line number
+Diff line change
@@ Expand Up / @@ -77,6 +77,9 @@ dependencies { @@
         testImplementation(libs.bundles.kotest)
         testImplementation(libs.mockk)
         testImplementation(libs.kotlinx.coroutines.test)
+        // jMolecules - DDD building blocks (needed for test compilation as tests use domain entities)
+        testImplementation(libs.jmolecules.ddd)
     }
     application {
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up @@
     import io.github.kamiazya.scopes.scopemanagement.domain.valueobject.AspectKey
     import io.github.kamiazya.scopes.scopemanagement.domain.valueobject.AspectValue
     import io.github.kamiazya.scopes.scopemanagement.domain.valueobject.Aspects
-    import io.github.kamiazya.scopes.scopemanagement.domain.valueobject.ScopeId
     import io.github.kamiazya.scopes.scopemanagement.domain.valueobject.ScopeTitle
     import io.github.kamiazya.scopes.scopemanagement.infrastructure.repository.InMemoryAspectDefinitionRepository
     import io.github.kamiazya.scopes.scopemanagement.infrastructure.repository.InMemoryScopeAliasRepository
@@ Expand Down Expand Up / @@ -113,8 +112,7 @@ class AspectQueryIntegrationTest : @@
                         aspectDefinitionRepository.save(timeDef)
                         // Create test scopes with aspects
-                        scope1 = Scope(
-                            id = ScopeId.generate(),
+                        scope1 = Scope.createForTest(
                             title = ScopeTitle.create("Task 1").getOrNull()!!,
                             description = null,
                             parentId = null,
@@ Expand All / @@ -137,8 +135,7 @@ class AspectQueryIntegrationTest : @@
                             aliasRepository = aliasRepository,
                         )
-                        scope2 = Scope(
-                            id = ScopeId.generate(),
+                        scope2 = Scope.createForTest(
                             title = ScopeTitle.create("Task 2").getOrNull()!!,
                             description = null,
                             parentId = null,
@@ Expand All / @@ -161,8 +158,7 @@ class AspectQueryIntegrationTest : @@
                             aliasRepository = aliasRepository,
                         )
-                        scope3 = Scope(
-                            id = ScopeId.generate(),
+                        scope3 = Scope.createForTest(
                             title = ScopeTitle.create("Task 3").getOrNull()!!,
                             description = null,
                             parentId = null,
@@ Expand All / @@ -185,8 +181,7 @@ class AspectQueryIntegrationTest : @@
                             aliasRepository = aliasRepository,
                         )
-                        scope4 = Scope(
-                            id = ScopeId.generate(),
+                        scope4 = Scope.createForTest(
                             title = ScopeTitle.create("Task 4").getOrNull()!!,
                             description = null,
                             parentId = null,
@@ Expand Down @@

-Original file line number
+Diff line change
@@ -1,7 +1,7 @@
     package io.github.kamiazya.scopes.apps.cli.test
+    import io.github.kamiazya.scopes.scopemanagement.domain.aggregate.ScopeAlias
     import io.github.kamiazya.scopes.scopemanagement.domain.entity.Scope
-    import io.github.kamiazya.scopes.scopemanagement.domain.entity.ScopeAlias
     import io.github.kamiazya.scopes.scopemanagement.domain.repository.ScopeAliasRepository
     import io.github.kamiazya.scopes.scopemanagement.domain.repository.ScopeRepository
     import io.github.kamiazya.scopes.scopemanagement.domain.valueobject.AliasName
@@ Expand Down @@

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

refactor: remove redundant jMolecules comments and improve ID extraction #293

Uh oh!

Diff view

Diff view

There are no files selected for viewing

coderabbitai bot Sep 30, 2025

Uh oh!

gemini-code-assist bot Sep 30, 2025

Uh oh!

Uh oh!

Uh oh!

-Original file line number
+Diff line change
@@ Expand Up / @@ -151,7 +151,7 @@ tasks.register("konsistTest") { @@
     configure<com.diffplug.gradle.spotless.SpotlessExtension> {
         kotlin {
             target("**/*.kt")
-            targetExclude("**/build/**/*.kt", "**/.tmp/**/*.kt")
+            targetExclude("**/build/**/*.kt", "**/.tmp/**/*.kt", "**/.gradle-local/**/*.kt")
             ktlint(
                 libs.versions.ktlint.tool
                     .get(),
@@ Expand Down Expand Up / @@ -194,7 +194,7 @@ configure<com.diffplug.gradle.spotless.SpotlessExtension> { @@
         }
         format("markdown") {
             target("**/*.md")
-            targetExclude("**/build/**/*.md")
+            targetExclude("**/build/**/*.md", "**/tmp/**/*.md")
             endWithNewline()
             // Trailing whitespace has semantic meaning in Markdown, so follow .editorconfig
         }
@@ Expand Down @@

-Original file line number
+Diff line change
@@ -1,17 +1,21 @@
     package io.github.kamiazya.scopes.devicesync.domain.entity
+    import io.github.kamiazya.scopes.devicesync.domain.valueobject.ConflictId
     import io.github.kamiazya.scopes.devicesync.domain.valueobject.ConflictType
     import io.github.kamiazya.scopes.devicesync.domain.valueobject.ResolutionAction
     import io.github.kamiazya.scopes.devicesync.domain.valueobject.VectorClock
     import kotlinx.datetime.Instant
+    import org.jmolecules.ddd.types.Entity
     /**
      * Represents a synchronization conflict with rich domain logic for resolution.
      *
      * This entity encapsulates the business rules for conflict detection, analysis,
      * and resolution strategies.
+     *
      */
     data class SyncConflict(
+        private val _id: ConflictId,
         val localEventId: String,
         val remoteEventId: String,
         val aggregateId: String,
@@ Expand All / @@ -23,7 +27,13 @@ data class SyncConflict( @@
         val detectedAt: Instant,
         val resolvedAt: Instant? = null,
         val resolution: ResolutionAction? = null,
-    ) {
+    ) : Entity<SyncState, ConflictId> {
+        /**
+         * Use getId() to access the conflict ID.
+         */
+        override fun getId(): ConflictId = _id
         init {
             require(localEventId.isNotBlank()) { "Local event ID cannot be blank" }
             require(remoteEventId.isNotBlank()) { "Remote event ID cannot be blank" }
@@ Expand Down Expand Up / @@ -180,6 +190,7 @@ data class SyncConflict( @@
                 require(remoteVersion >= 0) { "Remote version must be non-negative" }
                 return SyncConflict(
+                    _id = ConflictId.generate(),
                     localEventId = localEventId,
                     remoteEventId = remoteEventId,
                     aggregateId = aggregateId,
@@ Expand All / @@ -191,6 +202,37 @@ data class SyncConflict( @@
                     detectedAt = detectedAt,
                 )
             }
+            /**
+             * Create a new SyncConflict for testing purposes.
+             * Auto-generates an ID.
+             */
+            fun create(
+                localEventId: String,
+                remoteEventId: String,
+                aggregateId: String,
+                localVersion: Long,
+                remoteVersion: Long,
+                localVectorClock: VectorClock,
+                remoteVectorClock: VectorClock,
+                conflictType: ConflictType,
+                detectedAt: Instant,
+                resolvedAt: Instant? = null,
+                resolution: ResolutionAction? = null,
+            ): SyncConflict = SyncConflict(
+                _id = ConflictId.generate(),
+                localEventId = localEventId,
+                remoteEventId = remoteEventId,
+                aggregateId = aggregateId,
+                localVersion = localVersion,
+                remoteVersion = remoteVersion,
+                localVectorClock = localVectorClock,
+                remoteVectorClock = remoteVectorClock,
+                conflictType = conflictType,
+                detectedAt = detectedAt,
+                resolvedAt = resolvedAt,
+                resolution = resolution,
+            )
         }
     }
@@ Expand Down @@

-Original file line number
+Diff line change
@@ -0,0 +1,21 @@
+    @file:DomainLayer
+    package io.github.kamiazya.scopes.devicesync.domain
+    import org.jmolecules.architecture.layered.DomainLayer
+    /**
+     * Device Synchronization domain package.
+     *
+     * This bounded context handles multi-device consistency and synchronization.
+     * It models vector clocks, sync states, conflicts, and the synchronization protocol.
+     *
+     * Domain Types:
+     * - DeviceId: Identifier for devices (jMolecules Identifier)
+     * - SyncState: Entity tracking synchronization state per device
+     * - SyncConflict: Entity representing detected conflicts
+     * - VectorClock: Value object for causal ordering
+     *
+     * Domain Services:
+     * - DeviceSynchronizationService: Core sync logic
+     */

-Original file line number
+Diff line change
@@ -0,0 +1,14 @@
+    package io.github.kamiazya.scopes.devicesync.domain.valueobject
+    import io.github.kamiazya.scopes.platform.commons.id.ULID
+    import org.jmolecules.ddd.types.Identifier
+    /**
+     * Identity for SyncConflict entity.
+     */
+    @JvmInline
+    value class ConflictId(val value: String) : Identifier {
+        companion object {
+            fun generate(): ConflictId = ConflictId(ULID.generate().value)
+        }
+    }

-Original file line number
+Diff line change
@@ -1,15 +1,17 @@
     package io.github.kamiazya.scopes.devicesync.domain.valueobject
     import io.github.kamiazya.scopes.platform.commons.id.ULID
+    import org.jmolecules.ddd.types.Identifier
     /**
      * Represents a unique identifier for a device in the multi-device synchronization system.
      *
      * Each device participating in synchronization has a unique ID that is used to track
      * which events originated from which device and to maintain vector clocks.
+     *
      */
     @JvmInline
-    value class DeviceId(val value: String) {
+    value class DeviceId(val value: String) : Identifier {
         init {
             require(value.isNotBlank()) { "Device ID cannot be blank" }
             require(value.length <= 64) { "Device ID cannot exceed 64 characters" }
@@ Expand Down @@

Uh oh!

refactor: remove redundant jMolecules comments and improve ID extraction #293

Are you sure you want to change the base?

Uh oh!

refactor: remove redundant jMolecules comments and improve ID extraction #293

Uh oh!

Uh oh!

Diff view

Diff view

There are no files selected for viewing

coderabbitai bot Sep 30, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Sep 30, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!