Skip to content

Stop reading a rounded sixth decimal place as a decomposition bug - #545

Merged
blooop merged 1 commit into
mainfrom
fix/timing-total-rounding-flake
Sep 1, 2026
Merged

Stop reading a rounded sixth decimal place as a decomposition bug#545
blooop merged 1 commit into
mainfrom
fix/timing-total-rounding-flake

Conversation

@blooop

@blooop blooop commented Sep 1, 2026

Copy link
Copy Markdown
Owner

the_stages_account_for_no_more_than_the_total summed each stage's round6'd
seconds and compared that against a separately round6'd total with an exact
<=. Every number in the document is rounded on its own, so each part can round
up by half a microsecond while the whole rounds down by as much — the sum
then exceeds the total by arithmetic, with nothing charged twice, and the test
reported it as a decomposition bug.

It fired on #543 — Bump docker/login-action from 3 to 4,
a dependency bump that touches no Rust:

0.060256000000000004 against Document { total: 0.060256, stages: [
  StageReport { stage: "devpod-up", seconds: 0.030157, ... },
  StageReport { stage: "tools", seconds: 0.030099, ... }] }

What changed

The comparison now allows the precision the document is actually written to —
one half-microsecond per stage plus one for the total — rather than demanding
exactness the reporting never promised. Binary addition error is orders of
magnitude below that. A stage genuinely charged twice overshoots by a sleep, so
the check still catches what it exists for.

The comparison moved into stages_within_total so both sides of that boundary
get a deterministic test: the exact numbers from the failing run on one side, a
stage counted twice on the other. The flake is now a pinned regression instead
of a comment explaining a tolerance.

🤖 Generated with Claude Code

Summary by Sourcery

Make stage timing validation tolerate independent rounding while continuing to flag actual over-counting.

Bug Fixes:

  • Prevent rounded stage timings from being incorrectly reported as exceeding the rounded document total.
  • Retain detection of genuinely duplicated stage charges while allowing documented rounding precision.

Enhancements:

  • Extract the stage-versus-total validation into a reusable predicate with precision-based slack.

Tests:

  • Add regression coverage for the reported rounding-boundary failure and for detecting a stage counted twice.

`the_stages_account_for_no_more_than_the_total` compared a sum of
`round6`'d stage seconds against a separately `round6`'d total with an
exact `<=`. Each part can round up by half a microsecond while the whole
rounds down by as much, so the sum legitimately exceeds the total without
anything having been charged twice -- and the test called that a bug. It
failed on PR #543 with `0.030157 + 0.030099` against a total of
`0.060256`, a dependency bump that touches no Rust at all.

The comparison now allows the precision the document is written to: one
half-microsecond per stage plus one for the total. It moved into
`stages_within_total` so both sides of the boundary get a deterministic
test -- the numbers from the failing run on one side, a stage counted
twice on the other -- rather than waiting on a sleep to land badly.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Sorry @blooop, you've used your own review budget of 250,000 diff characters for the last 7 days.

You can request another review in 3 days and 9 hours by commenting @sourcery-ai review. Upgrade to get a review now.

@sourcery-ai

sourcery-ai Bot commented Sep 1, 2026

Copy link
Copy Markdown

Reviewer's Guide

The PR makes stage-total validation tolerant of the document’s independent six-decimal rounding while preserving detection of real stage overcounting, and adds deterministic tests for both boundaries using the exact regression values.

File-Level Changes

Change Details Files
Replace the exact stage-total comparison with a precision-derived tolerance that reflects independently rounded values.
  • Allow half a microsecond per reported stage plus one half-microsecond for the rounded total.
  • Keep the check focused on detecting meaningful overcharging rather than floating-point or display-rounding artifacts.
  • Move the comparison into a reusable helper used by the timing test.
rust/devlaunch-core/src/timing.rs
Add deterministic regression coverage for both rounding noise and genuine double-counting.
  • Pin the previously failing reported values as an accepted boundary case.
  • Verify that adding a duplicated stage still fails by a margin far beyond the tolerance.
  • Construct test documents directly to avoid timing-dependent sleeps.
rust/devlaunch-core/src/timing.rs

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@codecov

codecov Bot commented Sep 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.90%. Comparing base (c2b4ca7) to head (bc7c889).
⚠️ Report is 11 commits behind head on main.

Additional details and impacted files
Flag Coverage Δ
python 42.98% <ø> (ø)
rust 95.20% <100.00%> (-0.22%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
shipped code (rust) 95.20% <100.00%> (-0.22%) ⬇️
harness and tooling (python) 42.98% <ø> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@blooop
blooop merged commit c9910a1 into main Sep 1, 2026
15 checks passed
@blooop
blooop deleted the fix/timing-total-rounding-flake branch September 1, 2026 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant