fix(views): stop over-counting weekly unique for repeat same-week views#7761
Open
ar2rsawseen wants to merge 5 commits into
Open
fix(views): stop over-counting weekly unique for repeat same-week views#7761ar2rsawseen wants to merge 5 commits into
ar2rsawseen wants to merge 5 commits into
Conversation
…views Adds a views plugin test: one device views the same view in two sessions within a single ISO week. The view-data weekly-unique bucket (d.w<week>.u) must be 1, but live ingestion currently records 2 — it counts the same user's repeat same-week view twice. Regeneration (EE drill) already de-dupes correctly to 1; this test pins the live over-count in core where it actually lives. Expected to FAIL until the live counting is fixed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a regression test intended to reproduce a data-correctness bug in the Views plugin: weekly-unique view counting (d.w<week>.u) is over-counted when the same device views the same view in two sessions within one ISO week.
Changes:
- Add a new views ingestion test that sends two same-week sessions for one device/view and asserts weekly unique remains
1. - Compute a timezone-robust mid-week anchor timestamp for the previous ISO week.
- Query
app_viewsmetaandapp_viewdata*rollups to validate weekly-unique buckets.
The weekly-unique gate in recordMetrics built its comparison moment with new moment(lastViewTimestamp). lastViewTimestamp is a unix timestamp in seconds (as the surrounding day/month/year gates treat it), but moment(Number) interprets the value as milliseconds, placing it in 1970. isoWeek() of that 1970 date is a tiny number that is almost always less than the current weeklyISO, so the gate fired on every repeat view in the same week and incremented d.w<week>.u again -> weekly unique counted 2 for a single user. Daily/monthly/yearly gates were unaffected (they use elapsed-seconds comparisons). Use moment.unix() so isoWeek() reflects the real week. Also harden plugins/views/tests.js to assert a weekly-unique bucket was actually inspected (guards against a vacuous pass if weekly data stops being written). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ar2rsawseen
changed the title
Cookiezaurs
approved these changes
Jun 19, 2026
…de events Address review feedback: - The test was named plugins/views/tests.js, which Node resolves in preference to the existing plugins/views/tests/ directory, shadowing the whole views suite (tests/index.js -> views.js). Move it to plugins/views/tests/weeklyUnique.js and require it from index.js so the existing suite keeps running. - URL-encode the events JSON query param (encodeURIComponent) instead of concatenating raw JSON into the URL. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Completes the relocation: the new plugins/views/tests/weeklyUnique.js and its require in tests/index.js were missing from the previous commit (which only removed the shadowing plugins/views/tests.js). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Member
Author
|
Thanks — addressed all three:
|
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.
Summary
Fixes a data-correctness bug in live view ingestion: when one device views the same view in two sessions within a single ISO week, the view-data weekly-unique bucket (
d.w<week>.u) was recorded as 2 instead of 1.Root cause
In
recordMetrics(plugins/views/api/api.js), the weekly-unique gate built its comparison moment withnew moment(lastViewTimestamp).lastViewTimestampis a unix timestamp in seconds (the adjacent day/month/year gates compare it againsttimestamp - secInX), butmoment(Number)treats the value as milliseconds — placing it in 1970.isoWeek()of that 1970 date is a tiny number that is almost always< weeklyISO, so the gate fired on every repeat view in the same week and re-incrementedd.w<week>.u. Daily/monthly/yearly gates were unaffected (they use correct elapsed-seconds comparisons), which is why only weekly over-counted. Fix: usemoment.unix().Validation
plugins/views/tests.jsreproducing the scenario (same device, two same-week sessions). It failed on the first commit (d.w<week>.ucame back as 2) and passes with the fix.Note: this corrects go-forward live counting only; already-stored weekly
uvalues would need view-data regeneration to be corrected.🤖 Generated with Claude Code