Repository navigation
Conversation
Compare ordinary code units with charCodeAt and resolve full code points only at surrogate boundaries. Compute the first unequal encoded byte directly from code-point bit groups, removing the module-scoped scratch arrays and the helpers that fill them. Short-circuit equal strings and retain the comparator’s exact historical numeric results, including malformed UTF-16 and UTF-16 prefix-length behavior. Add reference-based coverage for encoding boundaries, compareValues results, paired and lone surrogates, every adjacent code-point pair, and deterministic arbitrary UTF-16 strings.
|
| Filename | Overview |
|---|---|
| npm-packages/convex/src/values/compare.ts | Routes strings exclusively through the UTF-8 comparator while retaining primitive comparison for bigint and boolean values. |
| npm-packages/convex/src/values/compare_utf8.ts | Replaces module-scoped byte buffers with direct UTF-16 scanning and bit-group comparison, with no concrete behavioral regression identified. |
| npm-packages/convex/src/values/compare_utf8.test.ts | Adds broad reference-based coverage for exact numeric results, encoding boundaries, paired and lone surrogates, and arbitrary UTF-16 strings. |
Reviews (1): Last reviewed commit: "values: remove shared UTF-8 comparison b..." | Re-trigger Greptile
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.
Motivation
Convex context reuse intentionally keeps a loaded module instance alive across multiple function invocations. That changes the lifetime of module-scope variables: top-level state is not discarded when one invocation returns, and can be observed by later invocations that reuse the instance. Request-derived state must therefore remain invocation-local. Otherwise the implementation has to prove, for every path and every future change, that data retained in module state can never affect or be exposed to another invocation.
compare_utf8currently keeps two UTF-8 scratch arrays at module scope. Each comparison writes bytes derived from its input strings into those arrays, and the bytes remain there after the call until a later comparison or module teardown. The comparator is synchronous, so this is not a data race, and the stale bytes are not currently observable in its result. It is nevertheless unnecessary cross-invocation retention of request-derived data. It lengthens the data lifetime beyond the invocation boundary and makes the context-reuse contract depend on a subtle overwrite proof rather than on the simpler invariant that invocation data does not escape into module state.Removing the arrays makes that invariant structural. The comparison state is local to the call, so it cannot survive an invocation, and future changes such as new early-return, error, reentrancy, or asynchronous call paths do not inherit a hidden module-level buffer to audit. This is a correctness and isolation improvement for reused module instances, not a claim that the old comparator currently returns incorrect values.
Summary
Remove the module-scoped UTF-8 scratch arrays. Scan ordinary UTF-16 code units with
charCodeAt, resolve full code points only at surrogate boundaries, and compute the first unequal UTF-8 byte directly from code-point bit groups. Also short-circuit equal strings.This makes comparison state invocation-local without replacing the shared buffers with per-comparison heap allocation. Avoiding encoded byte arrays and their writes also keeps the comparator hot path small. The implementation preserves the exact existing numeric results of the comparator, including its historical handling of malformed UTF-16 and UTF-16 prefix lengths.
Test plan
compareValuesresults.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.