Repository navigation
fix!: harden sort and BST contracts and enforce CI checks - #2
Merged
Merged
Conversation
Radix sort indexed its digit counts with (value / exp) % 10, which is negative for negative input and wrote before the stack count array. AddressSanitizer reported it as a stack-buffer-underflow. Take digits from each value's distance to the minimum, computed in unsigned arithmetic so INT_MIN..INT_MAX cannot overflow, and allocate the output buffer once instead of on every digit pass. Add tests for negative, all-negative, and extreme inputs, and a seeded randomized test that checks every sort against qsort.
Compare insert, delete, search, successor, predecessor, and k-th smallest against a presence array over 200 seeded trials. The xorshift generator makes failures reproducible on every platform.
Object rules depended only on their own source file, and the algorithm rule ignored headers entirely, so editing a header left stale objects. Generate dependency files with -MMD -MP and include them, which makes the hand-written graph object rules unnecessary. Declare test-bst, test-ht, and test-heap as phony targets.
Six files had drifted from gofmt output. Formatting only.
RadixSort indexed its digit counts with a negative value for negative input and panicked. CountingSort computed max-min+1 in int, which overflows for wide ranges, and allocated the whole range. Both now work on unsigned distances from the minimum. CountingSort falls back to slices.Sort when the range is more than 16 times the input length, matching the C implementation.
Fuzzing against reference models found three edge cases: - InterpolationSearch converted an int difference to float64, which overflows across the full int range, and divided by zero when the endpoint values were equal. The probe now uses unsigned distances. - BTree.Insert checked the wrong slot for an existing key stored in an internal node, so re-inserting that key added a second copy instead of updating its value. - RabinKarpSearch returned [0] and ZAlgorithmSearch returned every index but the last for an empty pattern. Both now return no matches, like KMPSearch.
BST keys are unique, but BSTFromSortedSlice built nodes straight from the slice, so sorted input with repeats produced duplicate keys and a wrong size. Drop adjacent repeats before building. The tests also cover Successor and Predecessor at the minimum, maximum, and for missing values.
Add native fuzz targets for the integer sorts, sorted-search helpers, string search, and the BST, AVL, red-black, B-tree, and skip-list implementations. Each is seeded with f.Add, so go test runs the seeds. Also cover the adjacency matrix, graph core accessors, LRU Clear, and empty queue paths.
Both sorts indexed their count arrays with negative values and threw ArrayIndexOutOfBoundsException. Counting sort also overflowed new int[max + 1] at Integer.MAX_VALUE. Radix sort now takes digits from each value's distance to the minimum, held in a long. Counting sort offsets by the minimum and falls back to Arrays.sort when the range is more than 16 times the input length. Every sort is now tested with negative and extreme inputs, and a seeded randomized test compares all of them with Arrays.sort.
insert sent duplicates to the right subtree while isValidBst used strict bounds, so inserting the same value twice made the tree report itself invalid. Inserting a present value now returns the existing node and leaves the tree unchanged, and fromSortedList keeps repeated values once. A seeded test compares the tree with TreeSet. BREAKING CHANGE: inserting a value that is already present no longer adds a node or increases size(). Callers that relied on storing duplicates should keep a count per value instead.
Several ASCII diagrams used a backslash followed by a space, an invalid escape that raises SyntaxWarning on Python 3.12+ and is scheduled to become an error. Others ended a line with a backslash, which joined the next line and garbled the diagram. Use raw docstrings where needed so the rendered text matches the source. Also correct the radix_sort docstring, which said it only handles non-negative integers.
insert stored duplicates in the right subtree while is_valid_bst used strict bounds, so inserting the same value twice made the tree report itself invalid. Inserting a present value now returns the existing node and leaves size and shape unchanged, and the factory methods drop repeated values. BREAKING CHANGE: inserting a value that is already present no longer adds a node or increases len(). Callers that relied on storing duplicates should keep a count per value instead.
AVLTree.insert(key, None) stored the key instead of None, because None doubled as the "no value given" default. Use a private sentinel so an omitted value still defaults to the key while an explicit None is kept. Also annotate with typing.Any instead of the builtin any, and repair the diagram escapes in this module.
lru_cache.py carried a second, smaller LFUCache. Re-export the full implementation from lfu_cache.py so both import paths name one class. Deleting the last entry at the minimum frequency left min_frequency() stale, so the next eviction looked in an empty bucket. Recompute the minimum in that case, which makes that delete O(n) in the worst case. The TTL cache tests move from test_lru_cache.py to their own file. BREAKING CHANGE: data_structures.lru_cache.LFUCache is now the lfu_cache class, so put() returns the evicted (key, value) pair instead of None.
Add fixed-seed randomized tests that compare ordered maps, hash tables, heaps, caches, range trees, sorts, searches, string matching, DP, union-find, and trie against simple reference models. Move the B-tree tests out of test_red_black_tree.py into test_b_tree.py, add red-black tree and graph core coverage, and set filterwarnings to error so an invalid escape sequence fails the suite.
CI ran clippy with -W, so about 30 lints accumulated, and rustdoc read index expressions such as dist[v] as broken intra-doc links. Apply clippy's suggested rewrites without changing behaviour (the Bloom filter keeps its NaN handling) and put index notation in code spans so cargo doc passes with -D warnings.
counting_sort allocated max + 1 counters, about 32 GB for u32::MAX, and counting_sort_i32 computed max - min + 1 in i32, which overflows for i32::MIN..i32::MAX. Compute ranges in a wider type, offset by the minimum, and fall back to sort_unstable when the range is more than 16 times the input length or the buffer cannot be allocated.
Compare the sorts, searches, string matching, union-find, trees, skip list, and caches against standard-library reference models. Failures report the seed, iteration, and operation history.
The workspace pointed at github.com/zenco/dsa instead of github.com/notzenco/dsa, and the dsa-data-structures crate docs listed a TTL cache that the Rust implementation does not have.
radixSort indexed its digit counts with negative digits, returned an all-negative array unsorted, and silently mis-sorted non-integers. Both sorts used Math.min and Math.max spreads, which throw on very large arrays. Radix sort now takes digits from each value's distance to the minimum, using BigInt so spans beyond 2^53 stay exact. Counting sort falls back to a comparison sort when the range is more than 16 times the input length. Both throw RangeError for values that are not safe integers.
insert sent duplicates right while isValid used strict bounds, so inserting the same value twice made the tree report itself invalid. Inserting a present value now returns the existing node and leaves the tree unchanged, and fromSortedArray keeps repeated values once. BREAKING CHANGE: inserting a value that is already present no longer adds a node or increases size. Callers that relied on storing duplicates should keep a count per value instead.
Add typescript as a dev dependency and a typecheck script that runs tsc --noEmit, so CI can enforce the strict tsconfig. Pin @types/bun to the locked 1.3.x line instead of latest.
Add a seeded PRNG and randomized tests that compare sorts, searches, string matching, trees, hash tables, heaps, caches, range trees, union-find, and trie against reference models. Cover the remaining B-tree deletion and heap branches.
CI did not enforce the checks REMEDIATION.md relies on. - C: add an AddressSanitizer and UBSan run that aborts on any report. - Rust: deny clippy warnings, check no_std and rustdoc, and check the library crates on the declared rust-version, 1.70. - Go: check gofmt and go vet, and test on 1.21 and current stable. - TypeScript: install from the frozen lockfile and type-check. - Java: use the committed Gradle wrapper instead of the runner's gradle. - Python: add 3.13 and 3.14 and fail on invalid escape sequences.
The hooks were committed without the executable bit, so the README's ./.githooks/install.sh failed. The commit-msg pattern rejected the ! breaking-change marker. Pre-commit ignored clippy failures; it now blocks on them and checks gofmt. Pre-push also runs Go, TypeScript, and Java when their toolchains are installed.
The README feature table claimed Rust has a min/max stack, circular queue, adjacency matrix, and Tarjan's SCC, and that TypeScript has two-pointer helpers; none of these exist. List Kosaraju's SCC and the Go-only modules, and say which languages carry diagrams and LeetCode mappings. The Rust README drops types it does not have and marks the trees without removal. Topic pages now link all six languages. REMEDIATION.md records the new sort, BST, LFU, B-tree, and Go search contracts and the full verification commands.
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.
Follow-up to the earlier audit. Fixes the remaining sort and BST boundary bugs across all six languages, adds randomized model tests, and makes CI enforce the checks
REMEDIATION.mdrelies on.Bug fixes
BTree.Insertadded a second copy of an existing key stored in an internal node instead of updating it.InterpolationSearchcould overflow across the fullintrange, and divided by zero when the endpoint values were equal.Nonevalue in an AVL tree was replaced by the key.min_frequency()went stale after deleting the last entry at the minimum frequency.SyntaxWarningand garbled several diagrams.Breaking changes
data_structures.lru_cache.LFUCacheis now a re-export ofdata_structures.lfu_cache.LFUCache, soput()returns the evicted(key, value)pair instead ofNone.countingSortandradixSortthrowRangeErrorfor values that are not safe integers.Tests, CI, and tooling
clippy -D warnings, rustdoc-D warnings, a no_std check, and a Rust 1.70 check of the library crates.gofmtandgo vet, tested on Go 1.21 and stable.tsctype-check.!breaking-change marker, pre-commit blocks on Clippy and gofmt, and pre-push covers every language.Docs
REMEDIATION.mdrecords the new contracts and verification commands.Verification
Run locally on macOS (arm64):
go test -race,go vet,gofmt, plus each fuzz target for ~6stscEvery commit on the branch passes its language's tests on its own.
Not verified locally: Go 1.21, Python 3.10–3.13 at runtime (the code was grammar-checked for 3.10), and Linux/gcc. This PR's CI covers them.
Known follow-ups (not in this PR)