Run track's optimistic metric update when the datastream is not connected - #115
Merged
Merged
Conversation
…cted track only bumped the cached company metrics when the datastream client reported connected. In replicator mode that meant usage stopped counting locally as soon as the replicator reported ready: false, even though it keeps serving its Redis cache and flag checks keep reading from it. The gate is removed so the local update runs whenever a datastream client is configured, in both replicator and WebSocket mode. The track event is still sent to the API either way. The server's company updates replace metric values rather than adding to them, so the local bump cannot double count, and updateCompanyMetrics is already a no-op for a company that is not cached. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
2 of 3 tasks
bpapillon
approved these changes
Sep 29, 2026
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
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.
Problem
tracksends the event to the API and then optimistically bumps the cached company metrics (DataStreamClient.updateCompanyMetrics) so local flag checks see the usage before the server's update arrives. That local update was gated ondataStreamClient.isConnected(). In replicator modeisConnected()is the replicator'sreadyfield, so as soon as the replicator reportedready: false(Schematic unreachable, account closed) usage stopped counting locally, even though the replicator keeps serving its Redis cache.Findings
Why the gate exists: it came in with the initial datastream/replicator port in #60, which has no description and no review discussion of the gate. It mirrors the Go SDK, where it was introduced in schematic-go#73 ("optimistic metric updates", no description) as
c.datastreamConnected, back when the only datastream mode was the direct WebSocket, and later rewritten toIsConnected()in schematic-go#82 without any stated reason. Nothing in the history points to a correctness requirement.Checked for correctness risks:
EntityMerge.upsertMetrics). Once the server has counted the event, its value replaces the local one.updateCompanyMetricsalready returns without doing anything if the company is not cached, and track still requires non-empty company keys.Change
Remove the
isConnected()condition, so the update runs whenever a datastream client is configured. This applies to both modes:Test
New
TrackOptimisticUpdateTest:trackbumps the matching cached metric (10 + 5 = 15) and leaves others alone.trackis a no-op for the cache and does not throw.The first two fail on
mainand pass with this change../gradlew compileJava spotlessCheck testpasses on JDK 11 (250 tests).🤖 Generated with Claude Code