feat(#79): GetPercentileAtOrBelowValue

[leecampbell-codeagent]

Issue #79: GetPercentileOfValue

Summary

The issue requests a method to answer: "Given a measurement value, what percentile does it fall under?"
This is the inverse of GetValueAtPercentile.
The API reference spec already names it GetPercentileAtOrBelowValue(long value) — that name is more precise and matches the Java HdrHistogram library's percentileAtOrBelowValue.
The method must be implemented in HistogramBase and exposed as a public instance method.

What Needs to Change and Why

HistogramBase currently has GetValueAtPercentile(double percentile) → long but no inverse.
Users recording response times need to ask: "300ms is at what percentile?" without iterating the distribution themselves.
The spec (spec/tech-standards/api-reference.md line 132) already lists GetPercentileAtOrBelowValue as part of the public API surface, so this is a missing implementation.

Affected Files (confirmed by exploration)

• HdrHistogram/HistogramBase.cs — add GetPercentileAtOrBelowValue(long value) method (near GetValueAtPercentile, around line 388)
• HdrHistogram.UnitTests/HistogramTestBase.cs — add shared tests covering all histogram types
• spec/tech-standards/api-reference.md — no change needed; method is already listed

Acceptance Criteria

1. A public method double GetPercentileAtOrBelowValue(long value) exists on HistogramBase.
2. Returns a value in the range [0.0, 100.0].
3. For a recorded value v, GetPercentileAtOrBelowValue(v) returns the percentage of all recorded values that are <= v (using the histogram's equivalent-value range semantics).
4. Returns 100.0 when value >= HighestTrackableValue or when value is beyond the tracked range.
5. Returns 0.0 when TotalCount == 0 (empty histogram).
6. Consistent with GetValueAtPercentile: GetValueAtPercentile(GetPercentileAtOrBelowValue(v)) should equal the highest equivalent value of the bucket containing v (within histogram resolution).
7. Monotonic: for v1 < v2, GetPercentileAtOrBelowValue(v1) <= GetPercentileAtOrBelowValue(v2).

Algorithm

Mirrors the bucket-traversal loop in GetValueAtPercentile but in the forward direction:

1. Compute targetBucketIndex = GetBucketIndex(value) and targetSubBucketIndex = GetSubBucketIndex(value, targetBucketIndex).
2. If targetBucketIndex >= BucketCount, return 100.0.
3. Accumulate counts for all sub-buckets up to and including (targetBucketIndex, targetSubBucketIndex).
4. Return (100.0 * countAtValue) / TotalCount.
5. Guard: if TotalCount == 0, return 0.0.

Test Strategy

Add tests to HistogramTestBase (runs across all histogram types via the abstract factory):

• Basic percentile: record a known set of values; assert GetPercentileAtOrBelowValue returns the expected percentage.
• Round-trip consistency: for a set of recorded values, assert GetValueAtPercentile(GetPercentileAtOrBelowValue(v)) equals HighestEquivalentValue(LowestEquivalentValue(v)).
• Empty histogram: TotalCount == 0 returns 0.0.
• At maximum: value at or above HighestTrackableValue returns 100.0.
• Monotonicity: verify that queried percentiles are non-decreasing for increasing input values.
• Single value: histogram with one recorded value; any query >= that value returns 100.0, any query < that value returns 0.0.
• Boundary: value exactly at LowestTrackableValue returns (count_at_lowest / TotalCount) * 100.

Tests should use FluentAssertions with appropriate tolerance for floating-point comparisons (e.g., BeApproximately(..., precision: 0.1)).

Risks and Open Questions

• Naming: The issue title says GetPercentileOfValue; the spec says GetPercentileAtOrBelowValue.
  Use GetPercentileAtOrBelowValue to match the spec and Java library.
• Values outside tracked range: The method should not throw; return 100.0 for values above HighestTrackableValue (consistent with Java implementation).
• Equivalent-value semantics: The query value is mapped to a bucket via GetBucketIndex/GetSubBucketIndex, matching the same resolution logic used throughout HistogramBase.
  All values within the same equivalent range map to the same percentile.
• Thread safety: The method only reads counts; concurrent histograms (LongConcurrentHistogram, IntConcurrentHistogram) inherit the same implementation and their atomic reads ensure a consistent (if momentarily stale) snapshot.

Task breakdown

Task Checklist: Issue #79 — GetPercentileAtOrBelowValue

Cross-reference: every acceptance criterion in brief.md is covered by at least one task below (see criterion labels in parentheses).

---

Implementation

• HdrHistogram/HistogramBase.cs — Add GetPercentileAtOrBelowValue(long value) as a public instance method returning double, placed directly after GetValueAtPercentile (around line 388).

  • Algorithm (mirrors the bucket-traversal loop of GetValueAtPercentile in the forward direction):
    1. Guard: if TotalCount == 0, return 0.0. (criterion 5)
    2. Compute bucketIndex = GetBucketIndex(value) and subBucketIndex = GetSubBucketIndex(value, bucketIndex).
    3. If bucketIndex >= BucketCount, return 100.0. (criterion 4)
    4. Traverse from bucket 0 / sub-bucket 0 through (bucketIndex, subBucketIndex) inclusive, accumulating counts via GetCountAt(i, j). Use the same loop structure as GetValueAtPercentile: first bucket starts at j = 0; subsequent buckets start at j = SubBucketHalfCount. (criterion 3)
    5. Return (100.0 * runningCount) / TotalCount, clamped to [0.0, 100.0]. (criteria 1, 2)
  • Verifiable: method appears in HistogramBase, compiles without error, and all tests below pass.

• HdrHistogram/HistogramBase.cs — Add XML doc comment for GetPercentileAtOrBelowValue following the existing style (summary, <param>, <returns>).

  • Must describe: the method is the inverse of GetValueAtPercentile; the return range [0.0, 100.0]; the equivalent-value-range semantics.
  • Verifiable: dotnet build produces no XML doc warnings for the new method.

---

Unit Tests

All tests go in HdrHistogram.UnitTests/HistogramTestBase.cs as [Fact] methods on the abstract base class, so they run for ShortHistogram, IntHistogram, and LongHistogram automatically.
Use FluentAssertions and the existing Create(highestTrackableValue, significantFigures) / Create(lowestTrackableValue, highestTrackableValue, significantFigures) factory helpers.
Floating-point comparisons must use .BeApproximately(..., precision: 0.1) unless an exact value is expected.

• Empty histogram returns 0.0 (criterion 5)

  • Create a histogram with no recorded values.
  • Assert histogram.GetPercentileAtOrBelowValue(TestValueLevel).Should().Be(0.0).
  • Verifiable: test is green for all three histogram word sizes.

• Value at or above HighestTrackableValue returns 100.0 (criterion 4)

  • Create a histogram, record at least one value.
  • Assert histogram.GetPercentileAtOrBelowValue(DefaultHighestTrackableValue).Should().Be(100.0).
  • Also assert for a value beyond range: histogram.GetPercentileAtOrBelowValue(long.MaxValue / 2).Should().Be(100.0).
  • Verifiable: test is green; no exception is thrown.

• Basic percentile — known value set (criterion 3)

  • Record values 1 through 100 (one each), so TotalCount == 100.
  • Assert GetPercentileAtOrBelowValue(50) is approximately 50.0 (±0.1).
  • Assert GetPercentileAtOrBelowValue(100) is approximately 100.0 (±0.1).
  • Assert GetPercentileAtOrBelowValue(1) is approximately 1.0 (±0.1).
  • Verifiable: test is green for all three histogram word sizes.

• Return value is always in range [0.0, 100.0] (criterion 2)

  • Record a known set of values.
  • For a range of query values (including 0, a recorded value, and HighestTrackableValue), assert each result .Should().BeInRange(0.0, 100.0).
  • Verifiable: test is green for all three histogram word sizes.

• Round-trip consistency with GetValueAtPercentile (criterion 6)

  • Record values 1, 10, 100, 1000.
  • For each recorded value v, compute p = histogram.GetPercentileAtOrBelowValue(v).
  • Assert histogram.GetValueAtPercentile(p) equals histogram.HighestEquivalentValue(histogram.LowestEquivalentValue(v)).
  • Verifiable: test is green; demonstrates the inverse relationship holds within histogram resolution.

• Monotonicity — non-decreasing percentiles for increasing values (criterion 7)

  • Record values 1, 50, 100, 500, 1000.
  • Query percentiles for 1, 10, 50, 100, 500, 1000, 5000 in order.
  • Assert each successive result is >= the previous one.
  • Verifiable: test is green for all three histogram word sizes.

• Single recorded value — queries below return 0.0, queries at or above return 100.0 (criteria 3, 4, 5)

  • Create a histogram, record value TestValueLevel (= 4) once.
  • Assert GetPercentileAtOrBelowValue(TestValueLevel - 1).Should().Be(0.0) (value below the recorded value).
  • Assert GetPercentileAtOrBelowValue(TestValueLevel).Should().Be(100.0) (value equals the recorded value).
  • Assert GetPercentileAtOrBelowValue(TestValueLevel + 1000).Should().Be(100.0) (value above the recorded value).
  • Verifiable: test is green for all three histogram word sizes.

• Boundary — query at LowestTrackableValue (criterion 3)

  • Create a histogram with lowestTrackableValue = 1.
  • Record value 1 once and value 1000 once (TotalCount == 2).
  • Assert GetPercentileAtOrBelowValue(1) is approximately 50.0 (±0.1) — one of two values is <= 1.
  • Verifiable: test is green for all three histogram word sizes.

---

Acceptance Criterion Coverage Matrix

Criterion	Covered by task(s)
1. Public method double GetPercentileAtOrBelowValue(long value) on HistogramBase	Implementation task (add method)
2. Return value in [0.0, 100.0]	Implementation (clamp); test "Return value is always in range"
3. Returns % of values <= v using equivalent-value semantics	Implementation (algorithm step 4); tests: basic percentile, single value, boundary
4. Returns 100.0 when value >= HighestTrackableValue or beyond range	Implementation (step 3); tests: at-maximum, single value (above)
5. Returns 0.0 when TotalCount == 0	Implementation (step 1); test: empty histogram
6. Round-trip: GetValueAtPercentile(GetPercentileAtOrBelowValue(v)) equals highest equivalent value of bucket	Test: round-trip consistency
7. Monotonic for increasing input values	Test: monotonicity

Closes #79

[LeeCampbell]

PR #135 Review: feat(#79): GetPercentileAtOrBelowValue

Author: leecampbell-codeagent | Base: main | Files: 2 (+151, -0)

---

Overview

This PR adds GetPercentileAtOrBelowValue(long value) to HistogramBase — the inverse of GetValueAtPercentile. Given a measurement value, it returns the percentile (0-100) at which that value falls in the distribution. This closes issue #79 and matches the already-documented method in spec/tech-standards/api-reference.md.

---

Implementation Review (HistogramBase.cs, +32 lines)

Algorithm correctness: Good. The implementation correctly mirrors the bucket-traversal loop in GetValueAtPercentile but in the forward direction:

1. Guard for TotalCount == 0 → returns 0.0
2. Computes target bucket/sub-bucket indices via GetBucketIndex/GetSubBucketIndex
3. Guard for bucketIndex >= BucketCount → returns 100.0
4. Accumulates counts from (0, 0) through (targetBucket, targetSubBucket)
5. Returns (100.0 * runningCount) / TotalCount

The inner loop start index (j = 0 for bucket 0, j = SubBucketCount / 2 for subsequent buckets) correctly handles the logarithmic bucketing scheme, consistent with GetValueAtPercentile at line 378.

Issues found:

1. Typo in branch name and PR title — GetPerecentileOfValue should be GetPercentileOfValue. The branch is agent/79-getperecentileofvalue. Not blocking but worth fixing the PR title.

2. Math.Min(100.0, ...) is unnecessary but harmless — Since runningCount <= TotalCount always holds (we only accumulate up to the target bucket), 100.0 * runningCount / TotalCount <= 100.0 is guaranteed. The Java percentileAtOrBelowValue omits this clamp. It's defensive and won't cause issues, but it's a minor divergence from the reference implementation.

3. No handling of negative input values — If value is negative, GetBucketIndex will compute a very large bucket index (because NumberOfLeadingZeros returns 0 for negative longs), which the bucketIndex >= BucketCount guard catches — returning 100.0. This is incorrect semantics: a negative value should logically return 0.0 (no recorded values can be below a negative value since the histogram only tracks non-negative values). The Java implementation has the same behaviour, so this is consistent, but worth being aware of. Consider adding a guard:

   if (value < 0) return 0.0;

4. XML doc is good — Follows existing <summary>, <param>, <returns> pattern. Minor note: the <param> tag lacks a period at the end, which is inconsistent with some other methods in the file, but this is a nit.

---

Test Review (HistogramTestBase.cs, +119 lines)

Coverage: Comprehensive. All 7 acceptance criteria are covered:

Criterion	Test(s)
1. Public method exists	All tests exercise it
2. Returns [0.0, 100.0]	ResultIsAlwaysInRange
3. Returns % of values <= v	KnownValueSet, SingleRecordedValue, BoundaryAtLowestTrackableValue
4. Returns 100.0 for values at/above max	ValueAtOrAboveHighestTrackable_Returns100
5. Returns 0.0 when empty	EmptyHistogram_ReturnsZero
6. Round-trip consistency	RoundTrip_ConsistentWithGetValueAtPercentile
7. Monotonicity	IsMonotonic

Plus a bonus test: LargeMagnitudeValues_ReturnsCorrectPercentile — tests values across different magnitude ranges. Good addition.

Issues found:

5. ResultIsAlwaysInRange has a redundant assertion — histogram.GetPercentileAtOrBelowValue(0).Should().Be(0.0) asserts outside the loop, but 0 is also included in the queryValue array inside the loop where it's checked with .BeInRange(0.0, 100.0). The standalone assertion is stricter (checks exact 0.0 vs range), so it's not exactly redundant, but the test would read more clearly if the exact-0.0 check was either inside the loop with a conditional or documented with a comment explaining why it's tested separately.

6. Missing edge case test: value = 0 — No test explicitly records values and then queries GetPercentileAtOrBelowValue(0). The ResultIsAlwaysInRange test queries 0 but only asserts it's 0.0 without context for why (no values recorded at 0). A dedicated test would strengthen coverage.

7. SingleRecordedValue tests assume TestValueLevel - 1 maps to a different bucket — TestValueLevel is 4. TestValueLevel - 1 is 3. Both values are small enough to be in bucket 0 but in different sub-buckets with 3 significant digits, so this works correctly. But the test would be more robust with a comment explaining this assumption.

8. RoundTrip test could be fragile with edge values — The round-trip asserts GetValueAtPercentile(p) == HighestEquivalentValue(LowestEquivalentValue(v)). This is correct for the histogram's equivalent-value semantics, but values at bucket boundaries could theoretically behave differently. The chosen test values (1, 10, 100, 1000) are well within safe ranges, so this is fine in practice.

---

Commit Hygiene

9. 5 commits with planning artifacts — The commit history includes:

   • plan(#79): initial brief from issue
   • plan(#79): review brief
   • plan(#79): create task breakdown
   • feat(#79): implement tasks
   • feat(#79): complete implementation

   The first three commits added/moved plan/ directory files that were subsequently cleaned up. The net result is clean (no plan files in the branch), but the commit history exposes internal workflow noise. Recommend squash-merging this PR to collapse into a single clean commit.

10. PR title typo — "GetPerecentileOfValue" → "GetPercentileOfValue" (extra 'e').

---

Summary

Category	Verdict
Correctness	The algorithm is correct and matches the Java reference implementation
Tests	Comprehensive — all acceptance criteria covered with good additional cases
Code style	Consistent with existing codebase
Documentation	XML doc present and adequate
PR hygiene	Needs title typo fix; recommend squash merge

Recommendation: Approve with minor fixes

The implementation is solid and well-tested. The blocking items before merge:

• Fix the PR title typo ("Perecentile" → "Percentile")
• Consider squash merge to clean up the planning commits

Optional improvements (non-blocking):

• Add a guard for negative input values (if (value < 0) return 0.0;)
• Add a test for GetPercentileAtOrBelowValue(0) on a histogram with recorded positive values
• Remove the unnecessary Math.Min(100.0, ...) clamp to match the Java implementation exactly (or keep it — it's harmless)