Skip to content

Commit 272614a

Browse files
committed
fix: address PR review comments for thread-safety and line number tracking
RendererModels.kt: - Replace mutable idCounter with random-based ID generation for thread-safety - Add documentation explaining copy() behavior with default parameters AndroidCodeParserFactory.kt: - Fix line number tracking in extractJvmImports, extractPythonImports, extractJsImports to use actual line numbers from match positions instead of sequential counters - Replace hashCode-based ID with deterministic composite ID (filePath:startLine:qualifiedName) - Add comment explaining the approximate endLine calculation for regex-based parsing
1 parent 7441c91 commit 272614a

File tree

2 files changed

+43
-14
lines changed

2 files changed

+43
-14
lines changed

mpp-core/src/androidMain/kotlin/cc/unitmesh/agent/tool/impl/AndroidCodeParserFactory.kt

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -83,40 +83,45 @@ private class AndroidCodeParser : CodeParser {
8383

8484
private fun extractJvmImports(content: String, filePath: String): List<ImportInfo> {
8585
val importRegex = Regex("""import\s+(static\s+)?([a-zA-Z_][\w.]*[\w*])""")
86-
return importRegex.findAll(content).mapIndexed { index, match ->
86+
return importRegex.findAll(content).map { match ->
87+
// Calculate actual line number from match position
88+
val lineNumber = content.substring(0, match.range.first).count { it == '\n' } + 1
8789
ImportInfo(
8890
path = match.groupValues[2].removeSuffix(".*"),
8991
type = ImportType.MODULE,
9092
filePath = filePath,
91-
startLine = index,
92-
endLine = index
93+
startLine = lineNumber,
94+
endLine = lineNumber
9395
)
9496
}.toList()
9597
}
9698

9799
private fun extractPythonImports(content: String, filePath: String): List<ImportInfo> {
98100
val imports = mutableListOf<ImportInfo>()
99-
var lineIndex = 0
100101

101102
val fromImportRegex = Regex("""from\s+([\w.]+)\s+import""")
102103
fromImportRegex.findAll(content).forEach { match ->
104+
// Calculate actual line number from match position
105+
val lineNumber = content.substring(0, match.range.first).count { it == '\n' } + 1
103106
imports.add(ImportInfo(
104107
path = match.groupValues[1],
105108
type = ImportType.MODULE,
106109
filePath = filePath,
107-
startLine = lineIndex++,
108-
endLine = lineIndex
110+
startLine = lineNumber,
111+
endLine = lineNumber
109112
))
110113
}
111114

112115
val importRegex = Regex("""^import\s+([\w.]+)""", RegexOption.MULTILINE)
113116
importRegex.findAll(content).forEach { match ->
117+
// Calculate actual line number from match position
118+
val lineNumber = content.substring(0, match.range.first).count { it == '\n' } + 1
114119
imports.add(ImportInfo(
115120
path = match.groupValues[1],
116121
type = ImportType.MODULE,
117122
filePath = filePath,
118-
startLine = lineIndex++,
119-
endLine = lineIndex
123+
startLine = lineNumber,
124+
endLine = lineNumber
120125
))
121126
}
122127

@@ -125,16 +130,17 @@ private class AndroidCodeParser : CodeParser {
125130

126131
private fun extractJsImports(content: String, filePath: String): List<ImportInfo> {
127132
val imports = mutableListOf<ImportInfo>()
128-
var lineIndex = 0
129133

130134
val es6ImportRegex = Regex("""import\s+(?:.+\s+from\s+)?['"]([@\w./-]+)['"]""")
131135
es6ImportRegex.findAll(content).forEach { match ->
136+
// Calculate actual line number from match position
137+
val lineNumber = content.substring(0, match.range.first).count { it == '\n' } + 1
132138
imports.add(ImportInfo(
133139
path = match.groupValues[1],
134140
type = ImportType.MODULE,
135141
filePath = filePath,
136-
startLine = lineIndex++,
137-
endLine = lineIndex
142+
startLine = lineNumber,
143+
endLine = lineNumber
138144
))
139145
}
140146

@@ -228,14 +234,18 @@ private class AndroidCodeParser : CodeParser {
228234
language: Language
229235
): CodeNode {
230236
val qualifiedName = if (packageName.isNotEmpty()) "$packageName.$name" else name
237+
// Use deterministic composite ID to avoid collisions
238+
val id = "$filePath:$startLine:$qualifiedName"
231239

232240
return CodeNode(
233-
id = qualifiedName.hashCode().toString(),
241+
id = id,
234242
type = type,
235243
name = name,
236244
packageName = packageName,
237245
filePath = filePath,
238246
startLine = startLine,
247+
// Approximate end line: regex parsing cannot determine actual end line,
248+
// so we use a reasonable default. For accurate end lines, use TreeSitter-based parsing.
239249
endLine = startLine + 10,
240250
startColumn = 0,
241251
endColumn = 0,

mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/render/RendererModels.kt

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,16 @@ enum class TaskStatus(val displayName: String) {
6161
/**
6262
* Base timeline item for chronological rendering.
6363
* This is the shared base class for timeline items in both ComposeRenderer and JewelRenderer.
64+
*
65+
* **Important**: When using `copy()` on data class instances, the `id` and `timestamp`
66+
* default parameters are NOT re-evaluated. This means copied items will retain the
67+
* original `id` and `timestamp` unless explicitly overridden:
68+
* ```kotlin
69+
* val item1 = TimelineItem.MessageItem(role = MessageRole.USER, content = "Hello")
70+
* val item2 = item1.copy(content = "World") // item2.id == item1.id (same ID!)
71+
* // To get a new ID:
72+
* val item3 = item1.copy(content = "World", id = TimelineItem.generateId())
73+
* ```
6474
*/
6575
sealed class TimelineItem(
6676
open val timestamp: Long = Platform.getCurrentTimestamp(),
@@ -165,8 +175,17 @@ sealed class TimelineItem(
165175
) : TimelineItem(timestamp, id)
166176

167177
companion object {
168-
private var idCounter = 0L
169-
fun generateId(): String = "${Platform.getCurrentTimestamp()}-${idCounter++}"
178+
/**
179+
* Thread-safe counter for generating unique IDs.
180+
* Uses timestamp + random component to avoid collisions across threads/instances.
181+
*/
182+
private val random = kotlin.random.Random
183+
184+
/**
185+
* Generates a unique ID for timeline items.
186+
* Uses timestamp + random component for thread-safety without requiring atomic operations.
187+
*/
188+
fun generateId(): String = "${Platform.getCurrentTimestamp()}-${random.nextInt(0, Int.MAX_VALUE)}"
170189
}
171190
}
172191

0 commit comments

Comments
 (0)