Fix inclusive bounds handling in Position encoding

[devin-ai-integration]

Fix inclusive bounds handling in Position encoding

Summary

Fixed a critical bug in the aggregate component where the inclusive argument for bounds was not being respected. The issue was in the Position encoding system in src/client/positions.ts where the Position type had an incorrect structure that wasn't properly encoding inclusive/exclusive boundary behavior.

Key Changes:

• Simplified Position type from [Key, string | null | never[], "" | null | never[]] to [Key, string | null | never[]]
• Fixed boundToPosition function to correctly encode inclusive/exclusive behavior using the ID field with BEFORE_ALL_IDS (null) and AFTER_ALL_IDS ([]) markers
• Added comprehensive test suite covering inclusive/exclusive bounds with scalar and array keys

Root Cause: The previous Position encoding used a third element to encode inclusive/exclusive behavior, but the comparison logic in the B-tree wasn't properly interpreting these markers, causing all bounds to be treated as inclusive.

Fix: Moved the inclusive/exclusive encoding into the ID field (second element) where the existing comparison logic already handles null vs [] ordering correctly.

Review & Testing Checklist for Human

• Verify array key handling - Test with complex array keys (nested arrays, mixed types, large arrays) to ensure the explodeKey/implodeKey logic works correctly with the new Position encoding
• Test boundary edge cases - Verify behavior with null keys, empty arrays, and boundary values like minimum/maximum numbers
• Performance validation - Run performance tests to ensure the simplified Position encoding doesn't impact B-tree operation speed
• Cross-reference Position usage - Review other files that use the Position type to ensure no breaking changes were introduced
• Integration testing - Test with real aggregate data and various query patterns beyond the unit test scenarios

Testing Plan

1. Run the new inclusive bounds test suite and verify all edge cases pass
2. Test with production-like data volumes and query patterns
3. Verify backward compatibility with any existing aggregate data
4. Test array key scenarios with various data types and nesting levels

---

Diagram

%%{ init : { "theme" : "default" }}%%
graph TD
    TestFile["src/client/index.test.ts<br/>Added comprehensive<br/>inclusive bounds tests"]:::major-edit
    PositionsFile["src/client/positions.ts<br/>Fixed Position type &<br/>boundToPosition encoding"]:::major-edit
    IndexFile["src/client/index.ts<br/>count/sum methods<br/>use boundsToPositions"]:::context
    BTreeFile["src/component/btree.ts<br/>filterBetween uses<br/>compareKeys for bounds"]:::context
    CompareFile["src/component/compare.ts<br/>compareValues handles<br/>null vs [] ordering"]:::context
    
    TestFile -->|"tests"| IndexFile
    IndexFile -->|"calls boundsToPositions"| PositionsFile
    PositionsFile -->|"creates Position tuples"| BTreeFile
    BTreeFile -->|"uses compareKeys"| CompareFile
    
    subgraph Legend
        L1[Major Edit]:::major-edit
        L2[Minor Edit]:::minor-edit  
        L3[Context/No Edit]:::context
    end

classDef major-edit fill:#90EE90
classDef minor-edit fill:#87CEEB
classDef context fill:#FFFFFF

Loading

Notes

• Session Info: Requested by Ian Macartney (ian@convex.dev) in Devin session: https://app.devin.ai/sessions/39a0826e15a642e2b78800f5d1a1a4f0
• Testing: All existing tests continue to pass, plus 5 new comprehensive test cases specifically for inclusive/exclusive bounds behavior
• Risk Level: Medium - This is a fundamental fix to a core data structure, but the change is well-isolated and thoroughly tested
• Backward Compatibility: Should be safe since the previous implementation was broken (all bounds were treated as inclusive)

[devin-ai-integration]

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

• Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
• Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

• Disable automatic comment and CI monitoring

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/get-convex/aggregate/@convex-dev/aggregate@70

commit: 38e1850