[Issue #2] Support kotlin multiplatform#7
Open
GusWard wants to merge 3 commits into
Open
Conversation
diffray
Bot
added
diffray-review-failed
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Owner
Author
|
@CodeRabbit review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (3)
blueprint-compose-preview-multiplatform/src/iosMain/kotlin/uk/co/gusward/blueprint/compose/preview/preview/SemanticsExtractor.kt (1)
8-13: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAvoid a permanent fallback loop on iOS unsupported extraction.
Because this actual always returns an empty map (Line 12),
BlueprintPreview’s empty-state branch is shown forever on iOS. Consider an explicit platform capability gate (e.g., expect/actual support flag) so unsupported platforms render content without the “refresh” prompt.🤖 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 `@blueprint-compose-preview-multiplatform/src/iosMain/kotlin/uk/co/gusward/blueprint/compose/preview/preview/SemanticsExtractor.kt` around lines 8 - 13, The rememberBlueprintItems function on iOS permanently returns an empty map, causing BlueprintPreview to display an empty state with a refresh prompt indefinitely. Create an explicit expect/actual platform capability flag (e.g., isBlueprintExtractionSupported) instead of always returning emptyMap in the iOS actual implementation. Then modify BlueprintPreview to check this flag and skip the empty-state branch on unsupported platforms, allowing content to render normally without the refresh prompt.blueprint-compose-preview-multiplatform/src/iosMain/kotlin/uk/co/gusward/blueprint/compose/preview/utils/NumberFormatter.kt (1)
3-4: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winNormalize iOS label formatting to match other targets.
Using
value.toString()here can produce long float strings and diverges from JVM’s0.##behavior, so blueprint size labels render inconsistently across platforms.💡 Suggested change
actual fun formatNumber(value: Float): String { - return value.toString() + val rounded = kotlin.math.round(value * 100f) / 100f + return rounded.toString().removeSuffix(".0") }🤖 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 `@blueprint-compose-preview-multiplatform/src/iosMain/kotlin/uk/co/gusward/blueprint/compose/preview/utils/NumberFormatter.kt` around lines 3 - 4, The formatNumber function in the iOS implementation currently uses value.toString() which produces inconsistent formatting across platforms compared to the JVM implementation that uses a 0.## pattern. Replace the value.toString() call with a Number formatting approach that matches the JVM's decimal format pattern (0.##) to ensure blueprint size labels render consistently across all platforms.blueprint-compose-preview-multiplatform/src/commonMain/kotlin/uk/co/gusward/blueprint/compose/preview/grid/utils/Arrows.kt (1)
13-18: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winReset the incoming Path before writing arrow geometry.
Because this API accepts a caller-provided
Path, current logic appends to any existing segments. Resetting first makes behavior deterministic.💡 Suggested change
) : Path = path.apply { + reset() val hypotenuseLength = min( lengthOfLine * 0.2f, 15f) val baseLength = min( lengthOfLine * 0.15f, 10f)🤖 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 `@blueprint-compose-preview-multiplatform/src/commonMain/kotlin/uk/co/gusward/blueprint/compose/preview/grid/utils/Arrows.kt` around lines 13 - 18, The function that creates arrow geometry in the apply block appends to the caller-provided Path without first clearing any existing segments, which makes behavior non-deterministic. Reset the incoming path at the very beginning of the apply block (before calculating hypotenuseLength and baseLength) to ensure the path is empty before writing the new arrow geometry. This ensures deterministic behavior regardless of what segments were previously in the path.
🤖 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
`@blueprint-compose-preview-multiplatform-core/src/commonMain/kotlin/uk/co/gusward/blueprint/compose/preview/grid/BlueprintLine.kt`:
- Around line 18-26: The hashCode method in the BlueprintLine class must be
modified to match the order-independent behavior of equals, which treats (start,
end) and (end, start) as equal. Update the hashCode implementation to use an
order-independent hash computation (such as XOR or addition) on the hash codes
of the start and end properties instead of their current order-dependent
calculation. Apply the same fix to the second location mentioned at lines 81-85.
- Line 66: The segment-intersection check on line 66 has a type mismatch where
Float values (numerator1 / denominator and numerator2 / denominator) are being
checked against a Double range (0.0..1.0). Change the range bounds from 0.0..1.0
to 0f..1f to create a ClosedFloatingPointRange<Float> that matches the Float
type of the values being checked with the in operator. This applies to both the
numerator1 check and the equivalent checks for numerator2 on the same line.
In
`@blueprint-compose-preview-multiplatform-core/src/commonMain/kotlin/uk/co/gusward/blueprint/compose/preview/grid/logic/BlueprintLineCalculator.kt`:
- Around line 50-54: The overlap detection in the filtering logic within the
BlueprintLineCalculator.kt file is using item origin positions (position.y)
instead of the actual candidate line segment bounds when checking overlap
windows. Replace the references to currentEntry.value.position.y and
other.position.y with the actual start and end coordinates of the candidate line
segment being evaluated. This same issue appears in multiple overlap checking
blocks throughout the file (at the locations mentioned in the comment), so apply
the same fix pattern consistently across all similar overlap window checks to
use the true line segment endpoints rather than just the item origins.
In
`@blueprint-compose-preview-multiplatform-no-op/src/commonMain/kotlin/uk/co/gusward/blueprint/compose/preview/preview/BlueprintPreviewNoOp.kt`:
- Around line 6-14: The no-op BlueprintPreview function is missing two
parameters that exist in the main implementation, breaking the drop-in
replacement contract. Add the missing parameters enabled: Boolean = true and
refreshIntervalMs: Long = 0L to the BlueprintPreview function signature. These
parameters should be included in the parameter list alongside backgroundAlpha,
contentAlpha, and showInternalItems to ensure the no-op implementation maintains
full API compatibility with the main implementation.
In
`@blueprint-compose-preview-multiplatform/src/androidMain/kotlin/uk/co/gusward/blueprint/compose/preview/preview/SemanticsExtractor.kt`:
- Around line 52-60: The catch block in the inspection mode extraction code
silently suppresses all exceptions without logging them, making it difficult to
diagnose preview extraction failures. Replace the empty catch block that catches
Exception with a logging statement that captures and logs the exception details,
ensuring that errors during extractBlueprintItemsFromSemantics or subsequent
operations are recorded for debugging purposes.
- Around line 148-332: The traverseSemanticsNode function has a try-catch block
with an empty catch handler that prevents child node traversal from occurring if
any exception is thrown while processing the current node. Move the child
traversal loop (the for loop that iterates through node.children and recursively
calls traverseSemanticsNode) outside and after the try-catch block so that even
if processing the current node fails, the function will still traverse and
process all child nodes in the subtree.
In
`@blueprint-compose-preview-multiplatform/src/commonMain/kotlin/uk/co/gusward/blueprint/compose/preview/grid/BlueprintGrid.kt`:
- Line 36: The staticGridSizeCache is a mutable map that accumulates entries
indefinitely without removing stale entries when preview instances are disposed,
causing a memory leak. Implement lifecycle-based eviction by adding a clearing
mechanism that removes entries from staticGridSizeCache when preview instances
are disposed. Consider adding a function to clear the cache or tie the cache
clearing to the disposal lifecycle of preview components, ensuring that disposed
preview instances do not leave stale entries in memory indefinitely.
- Line 53: The spacing calculation from gridSize.toPx() at line 53 in the
BlueprintGrid drawing logic does not guard against zero or negative values,
which causes the do/while loop (in the range 161-175) to become non-terminating
when start += spacing never advances. Add a guard to ensure spacing is always
positive by coercing the calculated spacing value to have a minimum positive
value, preventing UI thread lockups when gridSize becomes zero or negative.
In
`@blueprint-compose-preview-multiplatform/src/iosMain/kotlin/uk/co/gusward/blueprint/compose/preview/utils/CallSite.kt`:
- Around line 3-5: The getCallSiteId() function in CallSite.kt returns a
hardcoded "ios" string instead of a unique identifier per call site, causing
cache collisions across multiple iOS previews. Replace the hardcoded return
value with logic that inspects the current stack trace to extract a unique
identifier based on the calling function's class name and method name (similar
to how the Android implementation works), ensuring each preview function gets
its own distinct call-site ID for proper cache isolation.
In
`@blueprint-compose-preview-multiplatform/src/jvmMain/kotlin/uk/co/gusward/blueprint/compose/preview/utils/CallSite.kt`:
- Around line 5-9: The call-site ID returned in this function is too generic and
causes cache collisions across different preview invocations. To fix this,
include the line number from the stack trace element to create a more unique
identifier. Modify the return statement to concatenate the className,
methodName, and lineNumber from the stack trace element (accessed via
stackTrace[2].lineNumber) instead of just using className and methodName alone.
This will ensure each unique call site gets a distinct identifier and prevent
state reuse issues in downstream components that depend on stabilityId.
In
`@example-multiplatform/src/androidMain/kotlin/uk/co/gusward/example/multiplatform/compose/ExampleComplexDashboardComponentPreviews.kt`:
- Around line 9-11: The function ExampleComplexDashboardComponentNormalPreview()
in the androidMain preview file duplicates an identically named preview function
from the commonMain source set, causing a duplicate declaration compilation
error when Android merges both source sets. Rename the
ExampleComplexDashboardComponentNormalPreview() function in the androidMain file
to a unique name that reflects it is Android-specific (for example,
ExampleComplexDashboardComponentAndroidPreview or similar) to differentiate it
from the commonMain version.
In
`@example-multiplatform/src/androidMain/kotlin/uk/co/gusward/example/multiplatform/compose/ExampleFabComponentPreviews.kt`:
- Around line 9-11: The function ExampleFabComponentNormalPreview() is declared
in both the commonMain and androidMain source sets, causing a declaration clash
during Android compilation when both source sets are merged. Rename the
ExampleFabComponentNormalPreview() function in the androidMain source set to a
unique name that does not conflict with the one declared in commonMain, such as
ExampleFabComponentAndroidPreview() or
ExampleFabComponentNormalAndroidPreview().
---
Nitpick comments:
In
`@blueprint-compose-preview-multiplatform/src/commonMain/kotlin/uk/co/gusward/blueprint/compose/preview/grid/utils/Arrows.kt`:
- Around line 13-18: The function that creates arrow geometry in the apply block
appends to the caller-provided Path without first clearing any existing
segments, which makes behavior non-deterministic. Reset the incoming path at the
very beginning of the apply block (before calculating hypotenuseLength and
baseLength) to ensure the path is empty before writing the new arrow geometry.
This ensures deterministic behavior regardless of what segments were previously
in the path.
In
`@blueprint-compose-preview-multiplatform/src/iosMain/kotlin/uk/co/gusward/blueprint/compose/preview/preview/SemanticsExtractor.kt`:
- Around line 8-13: The rememberBlueprintItems function on iOS permanently
returns an empty map, causing BlueprintPreview to display an empty state with a
refresh prompt indefinitely. Create an explicit expect/actual platform
capability flag (e.g., isBlueprintExtractionSupported) instead of always
returning emptyMap in the iOS actual implementation. Then modify
BlueprintPreview to check this flag and skip the empty-state branch on
unsupported platforms, allowing content to render normally without the refresh
prompt.
In
`@blueprint-compose-preview-multiplatform/src/iosMain/kotlin/uk/co/gusward/blueprint/compose/preview/utils/NumberFormatter.kt`:
- Around line 3-4: The formatNumber function in the iOS implementation currently
uses value.toString() which produces inconsistent formatting across platforms
compared to the JVM implementation that uses a 0.## pattern. Replace the
value.toString() call with a Number formatting approach that matches the JVM's
decimal format pattern (0.##) to ensure blueprint size labels render
consistently across all platforms.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 27310a18-f022-4e05-b334-62af490af394
📒 Files selected for processing (48)
README.mdblueprint-compose-preview-multiplatform-core/build.gradle.ktsblueprint-compose-preview-multiplatform-core/src/commonMain/kotlin/uk/co/gusward/blueprint/compose/preview/constants/Direction.ktblueprint-compose-preview-multiplatform-core/src/commonMain/kotlin/uk/co/gusward/blueprint/compose/preview/grid/BlueprintLine.ktblueprint-compose-preview-multiplatform-core/src/commonMain/kotlin/uk/co/gusward/blueprint/compose/preview/grid/logic/BlueprintLineCalculator.ktblueprint-compose-preview-multiplatform-core/src/commonMain/kotlin/uk/co/gusward/blueprint/compose/preview/items/BlueprintItemData.ktblueprint-compose-preview-multiplatform-core/src/commonMain/kotlin/uk/co/gusward/blueprint/compose/preview/items/ParentConnectionConfig.ktblueprint-compose-preview-multiplatform-no-op/build.gradle.ktsblueprint-compose-preview-multiplatform-no-op/src/commonMain/kotlin/uk/co/gusward/blueprint/compose/preview/preview/BlueprintPreviewNoOp.ktblueprint-compose-preview-multiplatform/build.gradle.ktsblueprint-compose-preview-multiplatform/src/androidMain/kotlin/uk/co/gusward/blueprint/compose/preview/preview/SemanticsExtractor.ktblueprint-compose-preview-multiplatform/src/androidMain/kotlin/uk/co/gusward/blueprint/compose/preview/utils/CallSite.ktblueprint-compose-preview-multiplatform/src/androidMain/kotlin/uk/co/gusward/blueprint/compose/preview/utils/NumberFormatter.ktblueprint-compose-preview-multiplatform/src/commonMain/kotlin/uk/co/gusward/blueprint/compose/preview/constants/Color.ktblueprint-compose-preview-multiplatform/src/commonMain/kotlin/uk/co/gusward/blueprint/compose/preview/grid/BlueprintGrid.ktblueprint-compose-preview-multiplatform/src/commonMain/kotlin/uk/co/gusward/blueprint/compose/preview/grid/utils/Arrows.ktblueprint-compose-preview-multiplatform/src/commonMain/kotlin/uk/co/gusward/blueprint/compose/preview/grid/utils/Lines.ktblueprint-compose-preview-multiplatform/src/commonMain/kotlin/uk/co/gusward/blueprint/compose/preview/preview/BlueprintPreview.ktblueprint-compose-preview-multiplatform/src/commonMain/kotlin/uk/co/gusward/blueprint/compose/preview/preview/BlueprintTheme.ktblueprint-compose-preview-multiplatform/src/commonMain/kotlin/uk/co/gusward/blueprint/compose/preview/preview/ModifierExt.ktblueprint-compose-preview-multiplatform/src/commonMain/kotlin/uk/co/gusward/blueprint/compose/preview/preview/SemanticsExtractor.ktblueprint-compose-preview-multiplatform/src/commonMain/kotlin/uk/co/gusward/blueprint/compose/preview/utils/CallSite.ktblueprint-compose-preview-multiplatform/src/commonMain/kotlin/uk/co/gusward/blueprint/compose/preview/utils/NumberFormatter.ktblueprint-compose-preview-multiplatform/src/iosMain/kotlin/uk/co/gusward/blueprint/compose/preview/preview/SemanticsExtractor.ktblueprint-compose-preview-multiplatform/src/iosMain/kotlin/uk/co/gusward/blueprint/compose/preview/utils/CallSite.ktblueprint-compose-preview-multiplatform/src/iosMain/kotlin/uk/co/gusward/blueprint/compose/preview/utils/NumberFormatter.ktblueprint-compose-preview-multiplatform/src/jvmMain/kotlin/uk/co/gusward/blueprint/compose/preview/preview/SemanticsExtractor.ktblueprint-compose-preview-multiplatform/src/jvmMain/kotlin/uk/co/gusward/blueprint/compose/preview/utils/CallSite.ktblueprint-compose-preview-multiplatform/src/jvmMain/kotlin/uk/co/gusward/blueprint/compose/preview/utils/NumberFormatter.ktbuild.gradle.ktsexample-multiplatform/build.gradle.ktsexample-multiplatform/src/androidMain/kotlin/uk/co/gusward/example/multiplatform/compose/ExampleButtonComponentPreviews.ktexample-multiplatform/src/androidMain/kotlin/uk/co/gusward/example/multiplatform/compose/ExampleCheckboxComponentPreviews.ktexample-multiplatform/src/androidMain/kotlin/uk/co/gusward/example/multiplatform/compose/ExampleComplexDashboardComponentPreviews.ktexample-multiplatform/src/androidMain/kotlin/uk/co/gusward/example/multiplatform/compose/ExampleFabComponentPreviews.ktexample-multiplatform/src/androidMain/kotlin/uk/co/gusward/example/multiplatform/compose/ExampleRadioButtonComponentPreviews.ktexample-multiplatform/src/androidMain/kotlin/uk/co/gusward/example/multiplatform/compose/ExampleSliderComponentPreviews.ktexample-multiplatform/src/androidMain/kotlin/uk/co/gusward/example/multiplatform/compose/ExampleSwitchComponentPreviews.ktexample-multiplatform/src/androidMain/kotlin/uk/co/gusward/example/multiplatform/compose/ExampleTextFieldComponentPreviews.ktexample-multiplatform/src/commonMain/kotlin/uk/co/gusward/example/multiplatform/compose/ExampleButtonComponent.ktexample-multiplatform/src/commonMain/kotlin/uk/co/gusward/example/multiplatform/compose/ExampleCheckboxComponent.ktexample-multiplatform/src/commonMain/kotlin/uk/co/gusward/example/multiplatform/compose/ExampleComplexDashboardComponent.ktexample-multiplatform/src/commonMain/kotlin/uk/co/gusward/example/multiplatform/compose/ExampleFabComponent.ktexample-multiplatform/src/commonMain/kotlin/uk/co/gusward/example/multiplatform/compose/ExampleRadioButtonComponent.ktexample-multiplatform/src/commonMain/kotlin/uk/co/gusward/example/multiplatform/compose/ExampleSliderComponent.ktexample-multiplatform/src/commonMain/kotlin/uk/co/gusward/example/multiplatform/compose/ExampleSwitchComponent.ktexample-multiplatform/src/commonMain/kotlin/uk/co/gusward/example/multiplatform/compose/ExampleTextFieldComponent.ktsettings.gradle.kts
Comment on lines
+18
to
+26
| override fun equals(other: Any?): Boolean { | ||
|
|
||
| val otherLine = other as? BlueprintLine ?: return false | ||
|
|
||
| val offsetsMatchExactly = start == otherLine.start && end == otherLine.end | ||
| val offsetsMatchReversed = start == otherLine.end && end == otherLine.start | ||
|
|
||
| return offsetsMatchExactly || offsetsMatchReversed | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Make hashCode order-independent to match equals.
equals treats (start,end) and (end,start) as equal, but current hashCode does not. This violates the hash contract and breaks behavior in sets/maps.
Suggested fix
override fun hashCode(): Int {
- var result = start.hashCode()
- result = 31 * result + end.hashCode()
- return result
+ return start.hashCode() xor end.hashCode()
}Also applies to: 81-85
🤖 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
`@blueprint-compose-preview-multiplatform-core/src/commonMain/kotlin/uk/co/gusward/blueprint/compose/preview/grid/BlueprintLine.kt`
around lines 18 - 26, The hashCode method in the BlueprintLine class must be
modified to match the order-independent behavior of equals, which treats (start,
end) and (end, start) as equal. Update the hashCode implementation to use an
order-independent hash computation (such as XOR or addition) on the hash codes
of the start and end properties instead of their current order-dependent
calculation. Apply the same fix to the second location mentioned at lines 81-85.
| val numerator2 = (end.x - start.x) * (start.y - otherLine.start.y) - (end.y - start.y) * (start.x - otherLine.start.x) | ||
|
|
||
| // Check if intersection point is within line segments | ||
| return (numerator1 / denominator in 0.0..1.0 && numerator2 / denominator >= 0 && numerator2 / denominator <= 1) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical
🧩 Analysis chain
🏁 Script executed:
find . -name "BlueprintLine.kt" -type fRepository: GusWard/Blueprint-Compose-Preview
Length of output: 406
🏁 Script executed:
cat -n "blueprint-compose-preview-multiplatform-core/src/commonMain/kotlin/uk/co/gusward/blueprint/compose/preview/grid/BlueprintLine.kt"Repository: GusWard/Blueprint-Compose-Preview
Length of output: 4147
Use Float range bounds in segment-intersection checks.
Line 66 uses in 0.0..1.0 which creates a ClosedFloatingPointRange<Double>, but numerator1 / denominator and numerator2 / denominator are Float values. The in operator requires the element type to match the range type, causing a type mismatch in Kotlin.
Suggested fix
- return (numerator1 / denominator in 0.0..1.0 && numerator2 / denominator >= 0 && numerator2 / denominator <= 1)
+ val t = numerator1 / denominator
+ val u = numerator2 / denominator
+ return (t in 0f..1f && u in 0f..1f)📝 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
| return (numerator1 / denominator in 0.0..1.0 && numerator2 / denominator >= 0 && numerator2 / denominator <= 1) | |
| val t = numerator1 / denominator | |
| val u = numerator2 / denominator | |
| return (t in 0f..1f && u in 0f..1f) |
🤖 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
`@blueprint-compose-preview-multiplatform-core/src/commonMain/kotlin/uk/co/gusward/blueprint/compose/preview/grid/BlueprintLine.kt`
at line 66, The segment-intersection check on line 66 has a type mismatch where
Float values (numerator1 / denominator and numerator2 / denominator) are being
checked against a Double range (0.0..1.0). Change the range bounds from 0.0..1.0
to 0f..1f to create a ClosedFloatingPointRange<Float> that matches the Float
type of the values being checked with the in operator. This applies to both the
numerator1 check and the equivalent checks for numerator2 on the same line.
Comment on lines
+50
to
+54
| val overlaps = completedBlueprintLines.filter { | ||
| it.isVertical && it.start.x == verticalLineX && | ||
| maxOf(it.start.y, it.end.y) > minOf(currentEntry.value.position.y, other.position.y) && | ||
| minOf(it.start.y, it.end.y) < maxOf(currentEntry.value.position.y, other.position.y) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use actual candidate line endpoints when checking overlap windows.
These overlap checks use item origins (position.x / position.y) instead of the candidate line segment bounds. That can produce false overlap hits and unnecessary offset stacking for lines that do not actually overlap.
💡 Example fix pattern (apply similarly across all overlap checks)
- val overlaps = completedBlueprintLines.filter {
- it.isHorizontal && it.start.y == horizontalLineY &&
- maxOf(it.start.x, it.end.x) > minOf(currentEntry.value.position.x, other.position.x) &&
- minOf(it.start.x, it.end.x) < maxOf(currentEntry.value.position.x, other.position.x)
- }
+ val candidateStartX = currentEntry.value.position.x + if (isLeft) currentEntry.value.size.width else 0f
+ val candidateEndX = other.position.x + if (isLeft) 0f else other.size.width
+ val candidateMinX = minOf(candidateStartX, candidateEndX)
+ val candidateMaxX = maxOf(candidateStartX, candidateEndX)
+
+ val overlaps = completedBlueprintLines.filter {
+ it.isHorizontal && it.start.y == horizontalLineY &&
+ maxOf(it.start.x, it.end.x) > candidateMinX &&
+ minOf(it.start.x, it.end.x) < candidateMaxX
+ }Also applies to: 118-122, 167-170, 206-210, 245-249, 284-288
🤖 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
`@blueprint-compose-preview-multiplatform-core/src/commonMain/kotlin/uk/co/gusward/blueprint/compose/preview/grid/logic/BlueprintLineCalculator.kt`
around lines 50 - 54, The overlap detection in the filtering logic within the
BlueprintLineCalculator.kt file is using item origin positions (position.y)
instead of the actual candidate line segment bounds when checking overlap
windows. Replace the references to currentEntry.value.position.y and
other.position.y with the actual start and end coordinates of the candidate line
segment being evaluated. This same issue appears in multiple overlap checking
blocks throughout the file (at the locations mentioned in the comment), so apply
the same fix pattern consistently across all similar overlap window checks to
use the true line segment endpoints rather than just the item origins.
Comment on lines
+6
to
+14
| @Composable | ||
| fun BlueprintPreview( | ||
| backgroundAlpha: Float = 1.0f, | ||
| contentAlpha: Float = 1.0f, | ||
| showInternalItems: Boolean = false, | ||
| content: @Composable () -> Unit | ||
| ) { | ||
| content() | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Missing parameters in no-op BlueprintPreview signature—breaks drop-in replacement contract.
The no-op BlueprintPreview is missing two parameters that the main implementation has: enabled: Boolean = true and refreshIntervalMs: Long = 0L. When the no-op module is swapped in as a release-build dependency (as documented in the README), any code that explicitly passes these parameters will fail to compile.
For example, this would break in release builds:
BlueprintPreview(enabled = false, refreshIntervalMs = 500L) { content() }[critical_contract_mismatch]
🔧 Proposed fix: add missing parameters to no-op signature
`@Composable`
fun BlueprintPreview(
+ enabled: Boolean = true,
backgroundAlpha: Float = 1.0f,
contentAlpha: Float = 1.0f,
showInternalItems: Boolean = false,
+ refreshIntervalMs: Long = 0L,
content: `@Composable` () -> Unit
) {
content()
}🤖 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
`@blueprint-compose-preview-multiplatform-no-op/src/commonMain/kotlin/uk/co/gusward/blueprint/compose/preview/preview/BlueprintPreviewNoOp.kt`
around lines 6 - 14, The no-op BlueprintPreview function is missing two
parameters that exist in the main implementation, breaking the drop-in
replacement contract. Add the missing parameters enabled: Boolean = true and
refreshIntervalMs: Long = 0L to the BlueprintPreview function signature. These
parameters should be included in the parameter list alongside backgroundAlpha,
contentAlpha, and showInternalItems to ensure the no-op implementation maintains
full API compatibility with the main implementation.
Comment on lines
+52
to
+60
| if (isInspectionMode) { | ||
| try { | ||
| val immediateMap = extractBlueprintItemsFromSemantics(view, showInternalItems) | ||
| if (immediateMap.isNotEmpty() && immediateMap != blueprintItemDataState) { | ||
| blueprintItemDataState = immediateMap | ||
| staticBlueprintCache[stabilityId] = immediateMap | ||
| } | ||
| } catch (e: Exception) {} | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Log inspection-mode extraction failures instead of swallowing them.
Line 59 currently suppresses all exceptions silently, which makes preview extraction regressions very hard to diagnose.
🔧 Proposed fix
- } catch (e: Exception) {}
+ } catch (e: Exception) {
+ android.util.Log.w("PassiveBlueprint", "Inspection extraction failed", e)
+ }📝 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
| if (isInspectionMode) { | |
| try { | |
| val immediateMap = extractBlueprintItemsFromSemantics(view, showInternalItems) | |
| if (immediateMap.isNotEmpty() && immediateMap != blueprintItemDataState) { | |
| blueprintItemDataState = immediateMap | |
| staticBlueprintCache[stabilityId] = immediateMap | |
| } | |
| } catch (e: Exception) {} | |
| } | |
| if (isInspectionMode) { | |
| try { | |
| val immediateMap = extractBlueprintItemsFromSemantics(view, showInternalItems) | |
| if (immediateMap.isNotEmpty() && immediateMap != blueprintItemDataState) { | |
| blueprintItemDataState = immediateMap | |
| staticBlueprintCache[stabilityId] = immediateMap | |
| } | |
| } catch (e: Exception) { | |
| android.util.Log.w("PassiveBlueprint", "Inspection extraction failed", e) | |
| } | |
| } |
🧰 Tools
🪛 detekt (1.23.8)
[warning] 59-59: Empty catch block detected. If the exception can be safely ignored, name the exception according to one of the exemptions as per the configuration of this rule.
(detekt.empty-blocks.EmptyCatchBlock)
[warning] 59-59: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
🤖 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
`@blueprint-compose-preview-multiplatform/src/androidMain/kotlin/uk/co/gusward/blueprint/compose/preview/preview/SemanticsExtractor.kt`
around lines 52 - 60, The catch block in the inspection mode extraction code
silently suppresses all exceptions without logging them, making it difficult to
diagnose preview extraction failures. Replace the empty catch block that catches
Exception with a logging statement that captures and logs the exception details,
ensuring that errors during extractBlueprintItemsFromSemantics or subsequent
operations are recorded for debugging purposes.
Source: Linters/SAST tools
| content: @Composable () -> Unit | ||
| ) { | ||
|
|
||
| val spacing = LocalDensity.current.run { gridSize.toPx() } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Guard grid spacing to avoid a non-terminating draw loop.
If gridSize reaches 0.dp (or a negative value), spacing becomes <= 0, start += spacing never advances, and the do/while loop can lock the UI thread.
💡 Suggested change
- val spacing = LocalDensity.current.run { gridSize.toPx() }
+ val spacing = LocalDensity.current.run { gridSize.toPx() }
@@
Canvas(
modifier = Modifier.fillMaxSize(),
) {
+ if (spacing <= 0f) return@Canvas
val path = Path()
@@
- } while (start < size.width + size.height)
+ } while (start < maxOf(size.width, size.height))
}
}Also applies to: 161-175
🤖 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
`@blueprint-compose-preview-multiplatform/src/commonMain/kotlin/uk/co/gusward/blueprint/compose/preview/grid/BlueprintGrid.kt`
at line 53, The spacing calculation from gridSize.toPx() at line 53 in the
BlueprintGrid drawing logic does not guard against zero or negative values,
which causes the do/while loop (in the range 161-175) to become non-terminating
when start += spacing never advances. Add a guard to ensure spacing is always
positive by coercing the calculated spacing value to have a minimum positive
value, preventing UI thread lockups when gridSize becomes zero or negative.
Comment on lines
+3
to
+5
| actual fun getCallSiteId(): String { | ||
| return "ios" | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
iOS implementation does not provide unique call-site identification, causing cache collision across previews.
The hardcoded "ios" return value means all iOS preview functions will share the same stabilityId, which is used as the cache key in rememberBlueprintItems(). This causes blueprint overlay data from one preview to incorrectly appear on other iOS previews when multiple preview functions coexist in the same module.
The Android implementation correctly derives a unique identifier per callsite via stack trace inspection (className.methodName). iOS should use a similar approach to ensure proper cache isolation.
[major_issue]
💡 Proposed iOS implementation using stack trace
actual fun getCallSiteId(): String {
- return "ios"
+ return try {
+ val stackTrace = Throwable().stackTrace
+ if (stackTrace.size > 2) {
+ val element = stackTrace[2]
+ "${element.className}.${element.methodName}"
+ } else {
+ "unknown"
+ }
+ } catch (e: Exception) {
+ "ios_fallback"
+ }
}📝 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
| actual fun getCallSiteId(): String { | |
| return "ios" | |
| } | |
| actual fun getCallSiteId(): String { | |
| return try { | |
| val stackTrace = Throwable().stackTrace | |
| if (stackTrace.size > 2) { | |
| val element = stackTrace[2] | |
| "${element.className}.${element.methodName}" | |
| } else { | |
| "unknown" | |
| } | |
| } catch (e: Exception) { | |
| "ios_fallback" | |
| } | |
| } |
🤖 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
`@blueprint-compose-preview-multiplatform/src/iosMain/kotlin/uk/co/gusward/blueprint/compose/preview/utils/CallSite.kt`
around lines 3 - 5, The getCallSiteId() function in CallSite.kt returns a
hardcoded "ios" string instead of a unique identifier per call site, causing
cache collisions across multiple iOS previews. Replace the hardcoded return
value with logic that inspects the current stack trace to extract a unique
identifier based on the calling function's class name and method name (similar
to how the Android implementation works), ensuring each preview function gets
its own distinct call-site ID for proper cache isolation.
Comment on lines
+5
to
+9
| return if (stackTrace.size > 2) { | ||
| val element = stackTrace[2] | ||
| "${element.className}.${element.methodName}" | ||
| } else { | ||
| "unknown" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Call-site ID is too coarse for cache keys.
className.methodName (and "unknown") can collide across multiple preview invocations, which risks cross-instance cache/state reuse when downstream components key by stabilityId.
💡 Suggested change
actual fun getCallSiteId(): String {
val stackTrace = Throwable().stackTrace
return if (stackTrace.size > 2) {
val element = stackTrace[2]
- "${element.className}.${element.methodName}"
+ "${element.className}.${element.methodName}:${element.lineNumber}"
} else {
- "unknown"
+ "unknown:${stackTrace.firstOrNull()?.className ?: "no_stack"}"
}
}📝 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
| return if (stackTrace.size > 2) { | |
| val element = stackTrace[2] | |
| "${element.className}.${element.methodName}" | |
| } else { | |
| "unknown" | |
| return if (stackTrace.size > 2) { | |
| val element = stackTrace[2] | |
| "${element.className}.${element.methodName}:${element.lineNumber}" | |
| } else { | |
| "unknown:${stackTrace.firstOrNull()?.className ?: "no_stack"}" | |
| } |
🤖 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
`@blueprint-compose-preview-multiplatform/src/jvmMain/kotlin/uk/co/gusward/blueprint/compose/preview/utils/CallSite.kt`
around lines 5 - 9, The call-site ID returned in this function is too generic
and causes cache collisions across different preview invocations. To fix this,
include the line number from the stack trace element to create a more unique
identifier. Modify the return statement to concatenate the className,
methodName, and lineNumber from the stack trace element (accessed via
stackTrace[2].lineNumber) instead of just using className and methodName alone.
This will ensure each unique call site gets a distinct identifier and prevent
state reuse issues in downstream components that depend on stabilityId.
Comment on lines
+9
to
+11
| fun ExampleComplexDashboardComponentNormalPreview() { | ||
| ExampleComplexDashboardComponent() | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify duplicate top-level preview function names in commonMain and androidMain.
rg -n "fun ExampleComplexDashboardComponentNormalPreview\\s*\\(" example-multiplatform/src/commonMain/kotlin/uk/co/gusward/example/multiplatform/compose/ExampleComplexDashboardComponent.kt
rg -n "fun ExampleComplexDashboardComponentNormalPreview\\s*\\(" example-multiplatform/src/androidMain/kotlin/uk/co/gusward/example/multiplatform/compose/ExampleComplexDashboardComponentPreviews.ktRepository: GusWard/Blueprint-Compose-Preview
Length of output: 285
Rename the duplicate preview function to avoid Android compilation failure.
Line 9 duplicates ExampleComplexDashboardComponentNormalPreview() already declared in example-multiplatform/src/commonMain/kotlin/uk/co/gusward/example/multiplatform/compose/ExampleComplexDashboardComponent.kt (Line 176). When the Android target compiles, both commonMain and androidMain source sets are resolved together, causing a duplicate declaration error.
💡 Suggested fix
`@Preview`(showBackground = true, name = "1. ExampleComplexDashboardComponent (Normal)")
`@Composable`
-fun ExampleComplexDashboardComponentNormalPreview() {
+fun ExampleComplexDashboardComponentNormalAndroidPreview() {
ExampleComplexDashboardComponent()
}📝 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
| fun ExampleComplexDashboardComponentNormalPreview() { | |
| ExampleComplexDashboardComponent() | |
| } | |
| fun ExampleComplexDashboardComponentNormalAndroidPreview() { | |
| ExampleComplexDashboardComponent() | |
| } |
🤖 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
`@example-multiplatform/src/androidMain/kotlin/uk/co/gusward/example/multiplatform/compose/ExampleComplexDashboardComponentPreviews.kt`
around lines 9 - 11, The function
ExampleComplexDashboardComponentNormalPreview() in the androidMain preview file
duplicates an identically named preview function from the commonMain source set,
causing a duplicate declaration compilation error when Android merges both
source sets. Rename the ExampleComplexDashboardComponentNormalPreview() function
in the androidMain file to a unique name that reflects it is Android-specific
(for example, ExampleComplexDashboardComponentAndroidPreview or similar) to
differentiate it from the commonMain version.
Comment on lines
+9
to
+11
| fun ExampleFabComponentNormalPreview() { | ||
| ExampleFabComponent() | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify duplicate top-level preview function names in commonMain and androidMain.
rg -n "fun ExampleFabComponentNormalPreview\\s*\\(" example-multiplatform/src/commonMain/kotlin/uk/co/gusward/example/multiplatform/compose/ExampleFabComponent.kt
rg -n "fun ExampleFabComponentNormalPreview\\s*\\(" example-multiplatform/src/androidMain/kotlin/uk/co/gusward/example/multiplatform/compose/ExampleFabComponentPreviews.ktRepository: GusWard/Blueprint-Compose-Preview
Length of output: 258
Rename preview function to resolve declaration clash between source sets.
ExampleFabComponentNormalPreview() is already declared in commonMain at Line 30 of ExampleFabComponent.kt. Redeclaring it in androidMain causes a compile-time error when both source sets are merged during Android compilation.
Suggested fix
-fun ExampleFabComponentNormalPreview() {
+fun ExampleFabComponentNormalAndroidPreview() {
ExampleFabComponent()
}📝 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
| fun ExampleFabComponentNormalPreview() { | |
| ExampleFabComponent() | |
| } | |
| fun ExampleFabComponentNormalAndroidPreview() { | |
| ExampleFabComponent() | |
| } |
🤖 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
`@example-multiplatform/src/androidMain/kotlin/uk/co/gusward/example/multiplatform/compose/ExampleFabComponentPreviews.kt`
around lines 9 - 11, The function ExampleFabComponentNormalPreview() is declared
in both the commonMain and androidMain source sets, causing a declaration clash
during Android compilation when both source sets are merged. Rename the
ExampleFabComponentNormalPreview() function in the androidMain source set to a
unique name that does not conflict with the one declared in commonMain, such as
ExampleFabComponentAndroidPreview() or
ExampleFabComponentNormalAndroidPreview().
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds support for CMP view the following 4 new modules:
Initially these modules copy / duplicate the logic from the original Andorid module - eventually plan to consolidate the logic into the multiplatform core module, then migrate the main package to use KMP Android output under the hood so the project supports everything, but for safety just keeping it completely seperate for now while testing.
Summary by CodeRabbit
New Features
Documentation