feat(collections): add Trie<TValue> — ordered prefix tree with O(prefix) prefix search & longest-prefix match - #286
Merged
Merged
Conversation
Coverage
Files below 100% line coverage
|
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new Trie<TValue> collection to Celerity.Collections (string-keyed ordered prefix tree) along with parity rollout: tests, benchmarks, docs, website/dashboard wiring, and changelog/README updates.
Changes:
- Introduces
Trie<TValue>implementingIReadOnlyDictionary<string, TValue>with prefix enumeration and longest-prefix match APIs. - Adds a comprehensive xUnit test suite (core, prefix, enumeration, differential/oracle).
- Adds BenchmarkDotNet coverage + wires the collection into the benchmark dashboard and documentation.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| web/index.html | Adds “Trie” to the “What ships in the box” page. |
| web/dev/bench/index.html | Adds Trie to the benchmark collection list (ops: Add/Lookup/PrefixMatch). |
| web/dev/bench/detail.html | Adds Trie to the benchmark detail page collection list. |
| src/Celerity/Collections/Trie.cs | New Trie<TValue> implementation (ordered prefix tree). |
| src/Celerity.Tests/Collections/TrieTests.cs | Core behavior and exception-contract tests for Trie<TValue>. |
| src/Celerity.Tests/Collections/TriePrefixTests.cs | Tests for ContainsPrefix, GetByPrefix, TryGetLongestPrefix. |
| src/Celerity.Tests/Collections/TrieEnumerationTests.cs | Tests for ordering, IReadOnlyDictionary surface, and mutation-during-enumeration behavior. |
| src/Celerity.Tests/Collections/TrieDifferentialTests.cs | Differential randomized tests vs SortedDictionary oracle. |
| src/Celerity.Benchmarks/TrieBenchmark.cs | Adds Trie<int> vs Dictionary<string,int> benchmark, including prefix match workload. |
| src/Celerity.Benchmarks/Program.cs | Registers TrieBenchmark in the core benchmark suite. |
| README.md | Documents the new trie and adds usage/selection guidance. |
| docs/api/collections.md | Adds full API reference section for Trie<TValue>. |
| CHANGELOG.md | Adds [Unreleased] entries describing the new trie and rollout facets. |
Benchmarks20 regressions Highlights
Collections (396)
Hashers (100)
Same-runner A/B (sharded 6-way): main ( |
…ix) prefix search & longest-prefix match The BCL ships no trie. `Dictionary<string, TValue>` answers exact-key lookups but has no efficient prefix operation. `Trie<TValue>` maps string keys to values as a shared-prefix tree: - `GetByPrefix` / `GetKeysWithPrefix` — every entry under a prefix in O(prefix + matches), ascending key order (autocomplete, namespace/route listing). - `TryGetLongestPrefix` — the longest stored key that is a prefix of a query in O(query) (routing tables, tokenizer/dictionary matching). - Ordered iteration for free; implements IReadOnlyDictionary<string, TValue>. Nodes keep child edges in arrays sorted by edge char (binary-search lookup, ordered walk); removal prunes dead paths bottom-up. The empty string is a valid key. Not thread-safe. Full parity rollout in one PR: dedicated tests (Tests/Prefix/Enumeration/Differential vs a SortedDictionary ordinal oracle), TrieBenchmark + Program.cs registration, dashboard wiring (web/index.html + bench index/detail), docs (collections.md + README list/decision-table/ quick-start), and CHANGELOG. The int-keyed hash-family shared suites do not apply (same as Deque/DisjointSet); the roadmap has no Trie item to flip (post-roadmap tier-c enhancement). Closes #285. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
marius-bughiu
force-pushed
the
feat/issue-285-trie
branch
from
July 21, 2026 20:05
4420f1a to
1ea3aad
Compare
…lability, terse changelog - Snapshot the enumerator version at enumerable/enumerator creation (GetEnumerator / GetByPrefix / Keys / Values) and pass it into the DFS, so a mutation between handing out the sequence and the first MoveNext is detected on that first MoveNext (BCL-style), not one item late. Adds a leading version check + a covering test. - Annotate the non-interface out-params: Remove(out TValue?) and TryGetLongestPrefix(out string? key, out TValue? value), assigning plain null/default on the false path. TryGetValue keeps the IReadOnlyDictionary-matched signature (avoids CS8767). - Fix the misleading null-key exception in the bulk constructor and the stale "arrays are null" Node comment. - Condense the CHANGELOG [Unreleased] entry to one user-facing bullet per the CONTRIBUTING / CLAUDE.md brevity rule (the full rollout detail lives in the PR body). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ersion-check missing-prefix stream, doc nullability - TryGetLongestPrefix reuses the query string on an exact match and string.Empty on the empty-key match, so only a proper interior prefix allocates a Substring copy. - GetByPrefix now routes a missing prefix through the same version-checked walk (Enumerate accepts a nullable start), so an empty prefix result honours the enumerator-invalidation contract like a matching one instead of returning a bare Enumerable.Empty. - Update the docs method table to the actual out-param nullability (TryGetLongestPrefix / Remove: out string? / out TValue?). - Tests: exact/empty match reuse the source string (ReferenceEquals); a missing-prefix stream still throws when the trie is modified mid-enumeration. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
CHANGELOG.md:9
- The new changelog entry is quite long and reads more like a mini-design doc. CONTRIBUTING.md asks for changelog bullets to be “a few sentences at most” and user-facing (the release workflow copies the whole section into the GitHub Release body).
Please shorten this entry to the user-visible addition and keep the details in the PR description/docs.
- **`Trie<TValue>`** in `Celerity.Collections` — an ordered prefix tree mapping `string` keys to values, filling a BCL gap (.NET ships no trie). `GetByPrefix` lists every entry whose key starts with a prefix in `O(prefix + matches)` and in ascending key order, and `TryGetLongestPrefix` finds the longest stored key that is a prefix of a query in `O(query)` — the autocomplete, longest-prefix-routing, and ordered-iteration workloads a `Dictionary<string, TValue>` can only answer with an `O(n)` scan plus a `StartsWith` per key. Exact `Add` / `TryGetValue` favour a `Dictionary`, so the trie earns its place on the prefix operations. Implements `IReadOnlyDictionary<string, TValue>`; not thread-safe. Closes [#285](https://github.com/marius-bughiu/Celerity/issues/285).
…vention, drop LINQ - Implement IReadOnlyDictionary<string, TValue?> (was TValue), matching CelerityDictionary and the rest of the dictionary surface: public getters stay non-null (TValue), the nullable interface indexer is provided explicitly, and TryGetValue / Values / GetEnumerator now carry the TValue? annotation. Resolves the README "all dictionaries implement ...TValue?" tension. - Replace the two LINQ Select usages (GetKeysWithPrefix, Values) — the only LINQ in src/Celerity/Collections — with plain iterator helpers (EnumerateKeys / EnumerateValues), keeping the eager version snapshot. - Update docs/README to IReadOnlyDictionary<string, TValue?>. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…surface Follow-up to the IReadOnlyDictionary<string, TValue?> switch: the docs method table still showed the pre-change non-nullable signatures and the changelog still said IReadOnlyDictionary<string, TValue>. Align them: - TryGetValue -> out TValue? value - GetByPrefix -> IEnumerable<KeyValuePair<string, TValue?>> - Values -> IEnumerable<TValue?> - GetEnumerator -> IEnumerator<KeyValuePair<string, TValue?>> - CHANGELOG: implements IReadOnlyDictionary<string, TValue?> (The public indexer getter and the constructor input stay TValue, matching the code.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 6 comments.
Comments suppressed due to low confidence (1)
docs/api/collections.md:3522
Trie<TValue>.TryGetLongestPrefixis declared with nullableoutparameters (out string? key, out TValue? value). Usingout string route, out string handlerin the example implies non-nullable outs and will produce nullable-mismatch warnings for consumers who copy/paste. Update the example to usestring?outs to match the API.
if (routes.TryGetLongestPrefix("/api/v1/users/42", out string route, out string handler))
… leaf enumeration allocs - Test call sites and the README quick-start now declare the TryGetLongestPrefix key out-param as `string?` (matching the API), and the README example guards on the bool return — no more nullable-mismatch warnings and it demonstrates correct usage. - Enumerate short-circuits when the start node has no children (empty trie, or a prefix landing on a leaf key), skipping the StringBuilder + Stack allocation on those common small cases. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
src/Celerity/Collections/Trie.cs:465
Keys/Valuesare exposed asIEnumerable<...>sequences backed by iterator blocks, which means iterating them allocates an iterator state machine (and inKeysalso the traversal stack/StringBuilder used byEnumerate). In this codebase, dictionary-like collections typically expose lightweight structKeyCollection/ValueCollectionviews to avoid per-iteration allocations (e.g.CelerityDictionary’sKeys/Values). Consider adding non-allocating view types here too (even if traversal still needs a pooled stack), and keep theIReadOnlyDictionarymembers as explicit interface implementations if needed.
/// <summary>Gets the keys in ascending ordinal order.</summary>
public IEnumerable<string> Keys => GetKeysWithPrefix(string.Empty);
/// <summary>Gets the values ordered by their keys' ascending ordinal order.</summary>
public IEnumerable<TValue?> Values => EnumerateValues(_root, _version);
…line - GetEnumerator() now returns a public struct Enumerator (like Deque / DisjointSet / CelerityDictionary), so `foreach` over a Trie avoids the compiler-generated iterator state-machine allocation and enumerator boxing. The struct is the single traversal source: the internal Enumerate IEnumerable (for the GetByPrefix / Keys streams) now drives it, so the pre-order walk and modification detection live in one place. It lazily allocates its stack / StringBuilder only when the start node has children, preserving the leaf/empty fast path. Node is now `internal` (was private) so the public struct can name it as a ctor parameter. - TrieBenchmark: pre-size the Dictionary_Add baseline to ItemCount so the Add category measures per-insert cost rather than the dictionary's resize/rehash growth (matching sibling benchmarks). - Tests: struct-enumerator foreach / Reset / Dispose, single-leaf enumeration, and Reset-throws- on-modification. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
CHANGELOG.md:9
- The new CHANGELOG entry under [Unreleased] is much longer than the contributor guidance (“a few sentences at most”). Since the release workflow extracts the entire version section as the GitHub Release body, keeping this bullet tighter helps avoid release-size issues and keeps the changelog more scannable.
- **`Trie<TValue>`** in `Celerity.Collections` — an ordered prefix tree mapping `string` keys to values, filling a BCL gap (.NET ships no trie). `GetByPrefix` lists every entry whose key starts with a prefix in `O(prefix + matches)` and in ascending key order, and `TryGetLongestPrefix` finds the longest stored key that is a prefix of a query in `O(query)` — the autocomplete, longest-prefix-routing, and ordered-iteration workloads a `Dictionary<string, TValue>` can only answer with an `O(n)` scan plus a `StartsWith` per key. Exact `Add` / `TryGetValue` favour a `Dictionary`, so the trie earns its place on the prefix operations. Implements `IReadOnlyDictionary<string, TValue?>`; not thread-safe. Closes [#285](https://github.com/marius-bughiu/Celerity/issues/285).
…e API reference Mirror the note already in the type remarks: the O(prefix + matches) / O(query) forms count a character step as O(1), which is strictly O(log b) per node (constant for a bounded alphabet). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…eset to 0 The field is bumped on every post-construction mutation; the bulk-load constructor resets it to 0 after populating (no enumerator can exist during construction). Comment-only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
CHANGELOG.md:9
- This changelog bullet is much longer than the repo’s stated guideline (“a few sentences at most” and user-facing). Consider shortening it to a concise description of the new collection and its key capabilities, leaving the detailed rationale/complexity discussion to the PR description/docs.
- **`Trie<TValue>`** in `Celerity.Collections` — an ordered prefix tree mapping `string` keys to values, filling a BCL gap (.NET ships no trie). `GetByPrefix` lists every entry whose key starts with a prefix in `O(prefix + matches)` and in ascending key order, and `TryGetLongestPrefix` finds the longest stored key that is a prefix of a query in `O(query)` — the autocomplete, longest-prefix-routing, and ordered-iteration workloads a `Dictionary<string, TValue>` can only answer with an `O(n)` scan plus a `StartsWith` per key. Exact `Add` / `TryGetValue` favour a `Dictionary`, so the trie earns its place on the prefix operations. Implements `IReadOnlyDictionary<string, TValue?>`; not thread-safe. Closes [#285](https://github.com/marius-bughiu/Celerity/issues/285).
Match the actual API (public struct Enumerator, added in the struct-enumerator change) and the way every other collection's enumerator is documented in this file. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
CHANGELOG.md:9
- This
[Unreleased]changelog entry is significantly longer than the repo guideline (“a few sentences at most”); overlong entries can also break the release workflow that uses the full section as the GitHub Release body. Consider condensing to 1–2 user-facing sentences and leaving implementation/perf nuance to the PR description.
- **`Trie<TValue>`** in `Celerity.Collections` — an ordered prefix tree mapping `string` keys to values, filling a BCL gap (.NET ships no trie). `GetByPrefix` lists every entry whose key starts with a prefix in `O(prefix + matches)` and in ascending key order, and `TryGetLongestPrefix` finds the longest stored key that is a prefix of a query in `O(query)` — the autocomplete, longest-prefix-routing, and ordered-iteration workloads a `Dictionary<string, TValue>` can only answer with an `O(n)` scan plus a `StartsWith` per key. Exact `Add` / `TryGetValue` favour a `Dictionary`, so the trie earns its place on the prefix operations. Implements `IReadOnlyDictionary<string, TValue?>`; not thread-safe. Closes [#285](https://github.com/marius-bughiu/Celerity/issues/285).
…ter) Written as IReadOnlySet<,> (two params); the interface is IReadOnlySet<T>. Corrected to IReadOnlySet<>. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
CHANGELOG.md:9
- The new
[Unreleased]changelog bullet is longer than the contributor guideline (“a few sentences at most”) and increases the risk of release-body size issues (the release workflow uses the full section verbatim). Please condense to a short, user-facing summary of the observable capability and why it matters.
- **`Trie<TValue>`** in `Celerity.Collections` — an ordered prefix tree mapping `string` keys to values, filling a BCL gap (.NET ships no trie). `GetByPrefix` lists every entry whose key starts with a prefix in `O(prefix + matches)` and in ascending key order, and `TryGetLongestPrefix` finds the longest stored key that is a prefix of a query in `O(query)` — the autocomplete, longest-prefix-routing, and ordered-iteration workloads a `Dictionary<string, TValue>` can only answer with an `O(n)` scan plus a `StartsWith` per key. Exact `Add` / `TryGetValue` favour a `Dictionary`, so the trie earns its place on the prefix operations. Implements `IReadOnlyDictionary<string, TValue?>`; not thread-safe. Closes [#285](https://github.com/marius-bughiu/Celerity/issues/285).
4 tasks
marius-bughiu
added a commit
that referenced
this pull request
Jul 24, 2026
Resolves conflicts with the Trie (#286) and SparseSet (#288) merges, which touched the same shared parity files. Every conflict was a "keep both, in order" resolution — no content from either side was dropped: - Program.cs — CoreBenchmarks keeps Trie + Fenwick (SparseSet auto-merged) - web/index.html — ship cards for both Trie and FenwickTree - web/dev/bench/*.html — COLLECTIONS entries for both - README.md — both the "Prefix trees" and "Prefix sums" groups, both details blocks, and both decision-table rows (keeping main's updated iteration-order row that mentions Trie) - docs/api/collections.md — main's Trie section plus the FenwickTree section - CHANGELOG.md — all three Added entries plus main's new Fixed section Also condenses the FenwickTree changelog entry to a single user-facing bullet, matching the convention main just applied to the Trie and SparseSet entries. Full suite green after the merge: 4413 passed, 0 failed.
5 tasks
This was referenced Jul 25, 2026
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.
What
Adds
Trie<TValue>toCelerity.Collections— an ordered prefix tree mappingstringkeys to values, filling a genuine BCL gap (.NET ships no trie). Closes #285.Dictionary<string, TValue>answers exact-key lookups inO(1)but has no efficient prefix operation. The trie answers those directly from its structure:GetByPrefix/GetKeysWithPrefix— every entry whose key starts with a prefix, inO(prefix + matches)and ascending key order (autocomplete, typeahead, namespace/route listing). ADictionarymust scan every key and runStartsWith.TryGetLongestPrefix— the longest stored key that is a prefix of a query, inO(query)(routing tables, tokenizer/dictionary matching, longest-match).Dictionaryis unordered).Exact
Add/TryGetValuefavour aDictionary(one hash vs a per-character walk) — that's documented, and the prefix operations are the win (Guiding Principle: beats the BCL on ≥1 documented workload).Design. Each node keeps its child edges in two parallel arrays sorted by edge char, so a child lookup is a binary search and a pre-order walk is ordinal-ordered (sorted enumeration for free). Removal prunes dead paths bottom-up, so the structure never retains nodes that lead to no key. The empty string is a valid key. Implements
IReadOnlyDictionary<string, TValue>. Not thread-safe.Parity rollout (all in this PR)
src/Celerity/Collections/Trie.cs.TrieTests,TriePrefixTests,TrieEnumerationTests,TrieDifferentialTests(4,000-step randomized battery per seed vs aSortedDictionary<string,int>ordinal oracle), mirroring theDeque*/DisjointSet*four-file layout. 109 Trie tests; full suite 4255 passing.TrieBenchmark(Trie<int>vsDictionary<string,int>,Add/Lookup/PrefixMatch), registered inProgram.cs'sCoreBenchmarks.web/index.html;COLLECTIONSarrays inweb/dev/bench/index.htmlandweb/dev/bench/detail.html.docs/api/collections.md; README Collections list ("Prefix trees"), quick-start<details>, and a "Choosing a collection" row.[Unreleased].Parity facets that genuinely don't apply
AddAndTryAddTests,SetConstructorValidationTests,TryAddProbeCountTests, theIEnumerableConstructor*set, …) — those drive the int-keyed open-addressed hash-table contract (capacity / load-factor / probe-count); a string-keyed prefix tree has none of it, exactly likeDeque/DisjointSet.ROADMAP.md— noTrieline item to flip; the roadmap isdonethrough 2.1.0. This is a post-roadmap BCL-gap-filling enhancement (tier-c), the same pattern asDeque(Deque<T> — growable double-ended queue backed by a circular buffer #268) andDisjointSet(DisjointSet<T> — union-find (disjoint-set) for near-O(1) incremental connectivity #272).Test plan
dotnet build(whole solution, Debug) — 0 errors.dotnet test(net9.0) — 4255 passed, 0 failed (109 new Trie tests).main(the newTriecard renders oncedata.jspublishes).