Repository navigation
timeline digest: a count between 999,950 and 999,999 prints 1000.0k instead of 1.0M - #1784
Merged
Merged
Conversation
…ead of printing 1000.0k (ThinkFlowLab#1779) The k unit was picked from the raw value and the rounding happened inside it, so 999,950-999,999 printed 1000.0k, a number the unit cannot hold. Round once and carry into M, the way ThinkFlowLab#761 already carried a duration into the next unit.
Contributor
Author
|
Review request for this tip: |
Collaborator
|
Verdict: PASS at 8728d40 Ran at this tip: Review points:
Not run: |
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.
Fixes #1779
Owner: Dev2
Reviewer: Birfy
Written-by: Dev-2
Head:
8728d40f80f00ddff1737540f8b3414205d3e160(tree0715c7d2dd5c5b16894778e985d34bc1e9d8c404), base92a4cf262cb8f2469f800f12e5cfe006abb4d538.What the bug was
formatCount(plugins/timeline/src/digest.js) picked the unit from the raw value and rounded inside it, so any count between 999,950 and 999,999 rounded up to1000.0k— a number thekunit cannot hold, and one a reader has to convert back before it means anything.999,949read999.9k, then the count jumped to1000.0k, and only at 1,000,000 did it read1.0M. The same report already carried a duration that never overflows its unit (#761,1h60m), so the two formatters disagreed on the rule.Reproduction on the base
92a4cf26:node --input-type=module -e "import {formatCount} from './plugins/timeline/src/digest.js'; for (const n of [999949,999950,999999,1000000]) console.log(n, formatCount(n))"Red first
One new case in
plugins/timeline/test/digest.test.js, run at the base92a4cf262cb8with only the branch's test file in place andplugins/timeline/src/digest.jstaken from that base (cd plugins/timeline && node --test test/digest.test.js): 14 tests, 13 pass, 1 fail:The case pins all four boundaries as exact strings (
999,949 → 999.9k,999,950 → 1.0M,999,999 → 1.0M,1,000,000 → 1.0M), so it cannot pass on a substring or on a value that merely looks close. With the fix the same file is 14/14.Fix
formatCountnow rounds once and carries into the next unit, the wayformatMsalready does for durations:n >= Mkeeps printingMas before, so nothing at a million or above moves.kband the count is rounded to the unit's precision first (tenths = Math.round(n / 100)); when that rounding reaches10_000tenths, the count is one step below a million after rounding and prints(tenths / 10_000).toFixed(1) + 'M'.(n / K).toFixed(1)), so840,12.3k,1.2Mand the two negative/non-finite normalisations are byte-identical tomain. The carry only fires in the 999,950–999,999 slice.Mband has no unit above it in this formatter, so nothing there changes: no new unit, no different rounding, no behaviour change for any count below 999,950 or above 1,000,000.The subsystem page line for
plugins/timeline/src/digest.jsgains the count-carry rule next to the duration one, and a changelog fragment records the user-visible fix.Scope and lines
No generated file is in this diff: the change is in
plugins/timeline/, which is not covered bypatches/ds-bot/, so there is nocore/client.js, nopatches/ds-bot/*.patchand nofiles/mirror.plugins/timeline/src/digest.jsplugins/timeline/test/digest.test.jsdocs/subsystems/timeline.mdchangelog.d/1779-digest-count-rollover.fixed.md23 changed lines in total, well under the 150 stop-line, and no file outside
plugins/timeline/, its subsystem page andchangelog.d/.Check, twice on this tree
Both passes printed exactly those three lines; this diff touches no file under
core/orpatches/, sogeneratewas not run and no hunk was touched.Verify
Full verify at this head: core 2133, social 539, tree 125, timeline 130, workflow 289,
.github549, interop 37, live-e2e 24, every suite 0 fail;arch-docs: OK (4 changed file(s)).What I did not run
generateand nocore && npm run build: this branch changes no file undercore/, so there is no client bundle and no patch to regenerate (checkabove is the proof the tree already equalsds-bot/+ patches).dev/bench.sh smoke: noindex.jsand nosrc/host/change.plugins/timeline/is updated in this diff, so noDocs-not-needed:trailer is used.Gates at this head
Head:
8728d40f80f00ddff1737540f8b3414205d3e160(tree0715c7d2dd5c5b16894778e985d34bc1e9d8c404), base92a4cf262cb8f2469f800f12e5cfe006abb4d538.enqueue-guardon this head, verbatim:{"pr": 1784, "claim": "Dev2", "issue": 1779, "summary": "The team run report printed 1000.0k for a count between 999,950 and 999,999, because the unit was picked from the raw value and the rounding happened inside it. formatCount now rounds once and carries into the next unit, exactly as formatMs already does for durations: the slice reads 1.0M, and every count outside it keeps the expression it had. One new case pins all four boundaries as exact strings and was red at the base; full verify is green at the tip.", "files": ["plugins/timeline/src/digest.js", "plugins/timeline/test/digest.test.js", "docs/subsystems/timeline.md", "changelog.d/1779-digest-count-rollover.fixed.md"], "checks": {"ran": "TZ=UTC ARCH_DOCS_BASE=92a4cf262cb8 npm run verify", "result": "green", "at": "8728d40f80f00ddff1737540f8b3414205d3e160"}, "next": "none — ready for review"}