Send observation without reprs first, then reprs#120
Draft
Liam-DeVoe wants to merge 2 commits intomasterfrom
Draft
Send observation without reprs first, then reprs#120Liam-DeVoe wants to merge 2 commits intomasterfrom
Liam-DeVoe wants to merge 2 commits intomasterfrom
Conversation
Liam-DeVoe
force-pushed
the
incremental-reprs
branch
from
3e3b474 to
b50d8db
Compare
Zac-HD
reviewed
Jun 6, 2025
Owner
Zac-HD
left a comment
Zac-HD
left a comment
There was a problem hiding this comment.
I'm not convinced the latency win here is worth the code complexity, especially if we can get some streaming via #121. I think I'd rather get that in, then have the incremental upload prioritize tests where the page has been viewed, and then reassess whether we need this at all.
Comment on lines
+239
to
+240
| # using run_start as the observation primary key | ||
| "run_start": obs.run_start, |
Owner
There was a problem hiding this comment.
Makes me nervous; some clocks have pretty low resolution. Seems safe enough if we add the worker_uuid though.
Collaborator
Author
|
[drafting because I want to flesh out the dashboard pre-computation more, and make sure it's viable to defer observations like this] |
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
2 participants
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.
This doesn't buy us a whole lot right now, since the most prominent tyche chart is currently the mosaic of unique/duplicate, which relies on the repr. This does let other feature charts show earlier, though. And we might in the future precompute the unique/duplicate count on the backend.
This uses
run_startas the primary key to identify an observation, which makes me nervous but seems to be fine in practice?