Skip to content

fix(analytics): half-open trend buckets to stop boundary double-count - #520

Merged
JSONbored merged 3 commits into
JSONbored:mainfrom
philluiz2323:fix/quality-trend-bucket-boundaries
Jun 10, 2026
Merged

fix(analytics): half-open trend buckets to stop boundary double-count#520
JSONbored merged 3 commits into
JSONbored:mainfrom
philluiz2323:fix/quality-trend-bucket-boundaries

Conversation

@philluiz2323

Copy link
Copy Markdown
Contributor

Closes #519.

buildRecommendationQualityReportFromOutcomes builds two time-bucketed views over the same outcomes using the same contiguous trendPeriods boundaries (endMs[i] == startMs[i+1]), but with inconsistent interval semantics:

So an outcome exactly on an internal boundary was counted in two adjacent trend buckets but one rollup, inflating trends so their sum could exceed totals.total and disagree with the rollup table on the operator dashboard.

Example (generatedAt = 2026-06-01, windowDays = 14 -> split at 2026-05-25): one outcome at 2026-05-25T00:00:00.000Z gave trends = [1, 1] (sum 2) while totals.total = 1 and rollups count = 1.

Change

  • Apply the same half-open [start, end) rule (final bucket inclusive) in trendBuckets that qualityRollups already uses.
  • Add a regression test asserting a boundary outcome lands in exactly one bucket (trends = [0, 1], sum(trends) == totals.total, and the rollup is in the later bucket).

Verification

  • recommendation-quality-report.test.ts 10/10 (the new test fails on the old inclusive-end code). Full unit suite green; changed file 100% stmts/lines, branch coverage holds.

trendBuckets used an inclusive end (timestamp <= endMs) for every contiguous bucket, so an outcome exactly on an internal boundary was counted in two adjacent trend buckets -- inflating trends so their sum could exceed totals.total and disagree with qualityRollups. Use the same half-open [start, end) rule (final bucket inclusive) that qualityRollups already uses.
@ghost

ghost commented Jun 10, 2026

Copy link
Copy Markdown

Note

Gittensory Gate skipped

PR closed before full evaluation. No late first comment was created.

Signal Result Evidence Action
Gate result ⚠️ Skipped #520 is no longer open. No action.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

@ghost ghost added the gittensory:reviewed label Jun 10, 2026
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@JSONbored JSONbored added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jun 10, 2026

@JSONbored JSONbored left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@philluiz2323 this is good to land.

A few notes:

  • The half-open bucket change fixes the actual boundary double-count in trendBuckets.
  • The regression test covers the exact internal-boundary case and checks both trend totals and rollup alignment.
  • The missing formal Summary/Safety template sections are nonblocking here because the body explains the bug, change, and validation clearly enough.

No code changes requested.

@JSONbored
JSONbored merged commit 50ff4c8 into JSONbored:main Jun 10, 2026
6 checks passed
@github-project-automation github-project-automation Bot moved this from Todo to Done in gittensory - v1 roadmap Jun 10, 2026
@dosubot dosubot Bot added the size:XS label Jun 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

[Bug]: Recommendation-quality trends double-count outcomes exactly on bucket boundaries, while rollups count them once

2 participants