Conversation
📝 WalkthroughWalkthroughAdds Time‑To‑First‑State (TTFS) metrics: tracks whether a run started in the initial state and measures time to the first non‑initial state. Introduces schema v1.1, updates MetricSurface/CURRENT, extends StateMetrics snapshot, records TTFS in MetricsCollector, and conditionally emits the new gauges in sinks with tests and docs updated. Changes
Sequence Diagram(s)sequenceDiagram
participant Collector as MetricsCollector
participant Snapshot as MetricsSnapshot
participant Sink as MetricsSink
participant Exporter as ExternalExporter
Collector->>Collector: onStart(initialState)
Collector->>Collector: record startedInInitialState, start TTFS timer if needed
Collector->>Collector: onStateTransition(newState)
alt first non-initial transition
Collector->>Collector: record timeToFirstState
end
Collector->>Snapshot: build snapshot (includes TTFS fields)
Snapshot->>Sink: emit snapshot
Sink->>Exporter: publish metrics (conditionally include TTFS gauges if schema ≥ V1_1)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 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 |
Summary of ChangesHello @Nek-12, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the existing metrics collection system by adding new state-related metrics. Specifically, it introduces the ability to track whether a store begins its lifecycle in its initial state and, if so, how long it takes to transition to a different state. These additions provide deeper insights into state management performance and behavior, with careful consideration for schema versioning to maintain compatibility with existing metric consumers. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces new metrics for tracking the time to the first state transition and whether a store starts in its initial state. The changes are comprehensive, including updates to the metrics collector, data models, documentation, and metric sinks. The metrics schema has been versioned to 1.1, and sinks now conditionally expose these new metrics. The accompanying tests are thorough and cover the new functionality well. My review includes a correction for the metric count in the documentation and suggestions for more idiomatic Kotlin code in the metric sinks to improve readability. Overall, this is a well-executed feature addition.
metrics/src/commonMain/kotlin/pro/respawn/flowmvi/metrics/openmetrics/OpenMetricsSink.kt
Show resolved
Hide resolved
metrics/src/commonMain/kotlin/pro/respawn/flowmvi/metrics/otel/OtlpJsonSink.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@docs/docs/plugins/metrics.md`:
- Line 33: Update the metric count text that currently reads "Total: 67+ numeric
metrics per snapshot." to "Total: 68+ numeric metrics per snapshot." to account
for the two newly added metrics (started-in-initial-state and
time-to-first-state); locate the string "Total: 67+ numeric metrics per
snapshot." in the docs content and change the numeric value only.
In
`@metrics/src/commonMain/kotlin/pro/respawn/flowmvi/metrics/api/MetricsSnapshot.kt`:
- Around line 143-146: The new non-default properties on the public data class
are source-breaking; update the StateMetrics data class constructor to provide
defaults for the new parameters (e.g., startedInInitialState = false and
timeToFirstState = null) so existing callers compiling against the old
constructor keep working; adjust any internal factory/constructor calls that
assumed explicit values if needed, and ensure the property names
startedInInitialState and timeToFirstState are preserved for binary/source
compatibility.
In
`@metrics/src/commonMain/kotlin/pro/respawn/flowmvi/metrics/api/MetricsSnapshotExtensions.kt`:
- Around line 8-21: Update the KDoc for MetricsSnapshot.downgradeTo to reflect
its actual behavior: note that when target == meta.schemaVersion it returns
unchanged, but when downgrading to a version < MetricsSchemaVersion.V1_1 it
explicitly clears V1.1-specific fields (startedInInitialState set to false and
timeToFirstState set to null) in the returned snapshot; also mention that for
targets >= V1_1 the state is preserved and only meta.schemaVersion is updated.
metrics/src/commonMain/kotlin/pro/respawn/flowmvi/metrics/api/MetricsSnapshot.kt
Show resolved
Hide resolved
metrics/src/commonMain/kotlin/pro/respawn/flowmvi/metrics/api/MetricsSnapshotExtensions.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@metrics/src/commonMain/kotlin/pro/respawn/flowmvi/metrics/api/MetricsSnapshotExtensions.kt`:
- Around line 8-20: The function MetricsSnapshot.downgradeTo currently allows a
target schema newer than meta.schemaVersion; add a guard at the start of
MetricsSnapshot.downgradeTo that checks if target > meta.schemaVersion and in
that case do not change the snapshot (e.g., return this or throw a validation
error). Ensure the check uses the MetricsSchemaVersion comparison (target >
meta.schemaVersion) before any copy/update logic so you never mark a snapshot as
a higher schema without applying the corresponding upgrades.
Summary
Testing
Closes #190
Summary by CodeRabbit
New Features
Documentation
Tests