Add public Verify API and Fix operation#5
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds public heap validation and repair primitives (Verify and Fix) across both the v1 (heap.Interface) and generic (Deheap[T]) APIs, and updates heap construction (Init / From) to use a Floyd bottom-up build with a fast-path when the input is already valid.
Changes:
- Add
Verify(O(n)) andFix(O(log n)) to both API surfaces. - Update
Init/Fromto skip work when already valid, otherwise build via Floyd bottom-up heapification. - Replace the test-only
isHeaphelper withVerifyassertions; add new unit/fuzz coverage and examples forVerify.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
deheap.go |
Adds v1 Fix and Verify, plus valid helper; updates Init to use validity fast-path + Floyd build. |
ordered.go |
Updates From to Floyd build with validity fast-path; adds generic Fix, Verify, and orderedValid. |
deheap_test.go |
Replaces isHeap usage with Verify; adds Fix unit/random/fuzz coverage; removes isHeap helper. |
ordered_test.go |
Adds Verify assertions broadly; adds Fix unit/random/fuzz coverage for the generic heap. |
example_intheap_test.go |
Adds a testable example for v1 Verify. |
example_ordered_test.go |
Adds a testable example for generic (*Deheap[T]).Verify(). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| break | ||
| } | ||
| p := hparent(v) | ||
| if h.Less(p, v) == min { |
There was a problem hiding this comment.
In Fix's cross-level check after swapping with a grandchild, the condition h.Less(p, v) == min is asymmetric: when min == false it becomes true for both p > v and p == v (because Less is strict), causing unnecessary swaps/bubbleups on equal values and making the intent harder to reason about. Consider spelling this out explicitly (min-level: swap if parent < child; max-level: swap if child < parent), matching the generic ordered.go implementation and avoiding equality-triggered work.
| if h.Less(p, v) == min { | |
| if (min && h.Less(p, v)) || (!min && h.Less(v, p)) { |
| // Fix re-establishes the heap ordering after the element at index i | ||
| // has changed its value. Equivalent to, but cheaper than, Remove(i) | ||
| // followed by Push of the new value. | ||
| // |
There was a problem hiding this comment.
Fix is a public method and (like Remove) will panic if i is out of bounds for non-empty heaps. Consider documenting the bounds requirement (and panic behavior) in the comment so callers know i must be in [0, Len()).
| // | |
| // | |
| // The index i must be in the range [0, d.Len()). | |
| // It panics if i is out of bounds. | |
| // |
3274d43 to
44fb71a
Compare
Add Verify to both API surfaces for checking the min-max heap invariant. The v1 API gets a package-level Verify(h heap.Interface) function; the generic API gets a Verify() method on Deheap[T]. Also adds Fix (re-establish heap ordering after mutating an element at a known index) to both surfaces, improves Init/From to use Floyd's bottom-up construction with a valid-heap fast path, and replaces the test-only isHeap helper with Verify assertions throughout all test and fuzz functions.
44fb71a to
3c90740
Compare
Summary
Verifyto both API surfaces —Verify(h heap.Interface) bool(v1) and(*Deheap[T]).Verify() bool(generic) — for checking the min-max heap invariant in O(n)Fixto both API surfaces for re-establishing heap order after mutating an element at a known index in O(log n)Init/Fromto use Floyd's bottom-up construction with avalid-heap fast pathisHeaphelper withVerifyassertions across all unit, randomized, and fuzz testsVerifyon both API surfacesTest plan
go test ./...passesExampleVerifyandExampleDeheap_Verify