-
-
Notifications
You must be signed in to change notification settings - Fork 0
🐛 enhance Utils with overflow handling and list limit enforcement #21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthroughAdds overflow-aware list handling: combine/merge now accept a configurable Changes
Sequence Diagram(s)sequenceDiagram
participant Parser as Decoder parse step
participant Combine as Utils.combine(listLimit)
participant Overflow as Utils::Overflow helpers
participant Merge as Utils.merge
participant Final as Qs+Decode finalizer
Parser->>Combine: send parsed value(s) + listLimit
Combine->>Overflow: flatten inputs, check count
alt within limit
Combine-->>Parser: return array ([Any?])
else exceeds limit
Combine->>Overflow: build overflow dict, set maxIndex
Combine-->>Parser: return overflow dict
end
Parser->>Merge: merge combined item into accumulator
Merge->>Overflow: detect/propagate overflow flags & maxIndex
Merge-->>Parser: return merged (array/dict/overflow-aware dict)
Parser->>Final: deliver merged structure
Final->>Overflow: detect overflow map
alt is overflow map
Final->>Final: objectify (stringify numeric keys, remove overflow metadata)
Final-->>Output: emit objectified dict
else normal
Final-->>Output: emit structure as-is
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #21 +/- ##
==========================================
- Coverage 94.47% 93.37% -1.11%
==========================================
Files 53 54 +1
Lines 2663 2823 +160
==========================================
+ Hits 2516 2636 +120
- Misses 147 187 +40
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @Sources/QsSwift/Internal/Utils+Combine.swift:
- Around line 33-47: When combine(_:_:listLimit:) sees that first is an overflow
dictionary (isOverflow(dict)), appendOverflow(dict, value: second) currently
inserts second as a single element causing nested arrays; change the logic so
overflow merging preserves the same flattening semantics as appendCombineValue:
when appending into an overflow map, flatten arrays and append individual
elements (using appendCombineValue semantics), and represent nil as NSNull.
Update the overflow-append path (and the symmetric case around lines 68-79) to
iterate/flatten the incoming value and push elements into the overflow's
internal array rather than nesting the array as one element, ensuring
arrayToOverflowObject and isOverflow behavior remain consistent.
In @Sources/QsSwift/Internal/Utils+Overflow.swift:
- Around line 3-49: refreshOverflowMaxIndex currently only detects keys with
`key as? Int` and will miss bridged ObjC numeric keys (NSNumber); update
Utils.refreshOverflowMaxIndex to treat keys that are Int or NSNumber (e.g., if
let n = key as? Int ?? (key as? NSNumber)?.intValue) and compute the maxIndex
from those values, then call setOverflowMaxIndex(&dict, maxIndex) instead of
writing dict[overflowKey] directly to keep the write-path consistent; continue
skipping keys where isOverflowKey(key) is true.
🧹 Nitpick comments (2)
Sources/QsSwift/Internal/Utils+Combine.swift (1)
49-57: Avoid themap(Optional.some)allocation in a hot path.
arr.map(Optional.some)allocates an intermediate array; a simple loop can append without the extra allocation (especially relevant for Internal hot paths). As per coding guidelines, avoid unnecessary allocations in parsing/combining code.Possible tweak
} else if let arr = value as? [Any] { - array.append(contentsOf: arr.map(Optional.some)) + array.reserveCapacity(array.count + arr.count) + for v in arr { array.append(v) } }Sources/QsSwift/Internal/Utils+Merge.swift (1)
19-39: Overflow maxIndex should consider source’s declared overflowMaxIndex too (defensive).In the array→dict merge branch, when
sourceis overflow you setmaxIndexbased on observed keys. It’s safer to also takemax(sourceMax, computedMax)to guard against stale/malformed inputs (or future changes that don’t populate all numeric keys). Based on learnings, keep max-index logic consistent.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
Sources/QsSwift/Internal/Decoder+ParseObject.swiftSources/QsSwift/Internal/Decoder+ParseQuery.swiftSources/QsSwift/Internal/Utils+Combine.swiftSources/QsSwift/Internal/Utils+Merge.swiftSources/QsSwift/Internal/Utils+Overflow.swiftSources/QsSwift/Internal/Utils.swiftSources/QsSwift/Qs+Decode.swiftTests/QsObjCTests/ObjCDecodeTests.swiftTests/QsSwiftTests/DecodeTests.swiftTests/QsSwiftTests/UtilsTests.swift
🧰 Additional context used
📓 Path-based instructions (8)
Tests/**/*.swift
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Preserve deterministic ordering in fixtures and tests by using OrderedDictionary; do not replace with [String: Any] where ordering matters
Files:
Tests/QsObjCTests/ObjCDecodeTests.swiftTests/QsSwiftTests/DecodeTests.swiftTests/QsSwiftTests/UtilsTests.swift
Tests/QsObjCTests/**/*.{m,mm,swift}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Tests/QsObjCTests/**/*.{m,mm,swift}: Add unit tests in QsObjCTests when features are bridged to ObjC
In ObjC tests, do not rely on NSDictionary enumeration order; explicitly sort when checking key order
Files:
Tests/QsObjCTests/ObjCDecodeTests.swift
Tests/**/*.{swift,m,mm}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Prefer explicit assertion helpers over broad equality when validating ordering or null semantics
Files:
Tests/QsObjCTests/ObjCDecodeTests.swiftTests/QsSwiftTests/DecodeTests.swiftTests/QsSwiftTests/UtilsTests.swift
**/*.{swift,h,m,mm}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use 4-space indentation across Swift and Objective-C sources
Files:
Tests/QsObjCTests/ObjCDecodeTests.swiftSources/QsSwift/Internal/Utils.swiftSources/QsSwift/Internal/Decoder+ParseQuery.swiftSources/QsSwift/Internal/Utils+Overflow.swiftSources/QsSwift/Internal/Utils+Merge.swiftTests/QsSwiftTests/DecodeTests.swiftTests/QsSwiftTests/UtilsTests.swiftSources/QsSwift/Qs+Decode.swiftSources/QsSwift/Internal/Decoder+ParseObject.swiftSources/QsSwift/Internal/Utils+Combine.swift
**/*.swift
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Doc comments should be concise and note option interactions (e.g., decodeDotInKeys implies allowDots)
Files:
Tests/QsObjCTests/ObjCDecodeTests.swiftSources/QsSwift/Internal/Utils.swiftSources/QsSwift/Internal/Decoder+ParseQuery.swiftSources/QsSwift/Internal/Utils+Overflow.swiftSources/QsSwift/Internal/Utils+Merge.swiftTests/QsSwiftTests/DecodeTests.swiftTests/QsSwiftTests/UtilsTests.swiftSources/QsSwift/Qs+Decode.swiftSources/QsSwift/Internal/Decoder+ParseObject.swiftSources/QsSwift/Internal/Utils+Combine.swift
Sources/QsSwift/Internal/**/*.swift
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Sources/QsSwift/Internal/**/*.swift: In hot paths (parsing/encoding engine), avoid unnecessary allocations and do not introduce new regex use unless already present
If sort closure is nil and encode=false, retain traversal order; maintain stability when adding traversal logic
For large indices above listLimit, convert arrays to maps and keep logic consistent
Depth and parameter limits must remain O(length(query)); avoid quadratic regressions
Files:
Sources/QsSwift/Internal/Utils.swiftSources/QsSwift/Internal/Decoder+ParseQuery.swiftSources/QsSwift/Internal/Utils+Overflow.swiftSources/QsSwift/Internal/Utils+Merge.swiftSources/QsSwift/Internal/Decoder+ParseObject.swiftSources/QsSwift/Internal/Utils+Combine.swift
Sources/QsSwift/**/*.swift
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Sources/QsSwift/**/*.swift: Respect null semantics: use NSNull() for null-like values and Undefined() to omit keys; honor strictNullHandling and skipNulls flags
Do not swallow errors in core encode/decode; propagate as thrown Swift errors
Files:
Sources/QsSwift/Internal/Utils.swiftSources/QsSwift/Internal/Decoder+ParseQuery.swiftSources/QsSwift/Internal/Utils+Overflow.swiftSources/QsSwift/Internal/Utils+Merge.swiftSources/QsSwift/Qs+Decode.swiftSources/QsSwift/Internal/Decoder+ParseObject.swiftSources/QsSwift/Internal/Utils+Combine.swift
Tests/QsSwiftTests/**/*.swift
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Add unit tests in QsSwiftTests for new Swift options/behaviors
Files:
Tests/QsSwiftTests/DecodeTests.swiftTests/QsSwiftTests/UtilsTests.swift
🧠 Learnings (21)
📓 Common learnings
Learnt from: CR
Repo: techouse/qs-swift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T21:33:37.731Z
Learning: Applies to Sources/QsSwift/Internal/**/*.swift : For large indices above listLimit, convert arrays to maps and keep logic consistent
📚 Learning: 2025-10-13T21:33:37.731Z
Learnt from: CR
Repo: techouse/qs-swift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T21:33:37.731Z
Learning: Applies to Tests/QsObjCTests/**/*.{m,mm,swift} : Add unit tests in QsObjCTests when features are bridged to ObjC
Applied to files:
Tests/QsObjCTests/ObjCDecodeTests.swiftTests/QsSwiftTests/DecodeTests.swiftTests/QsSwiftTests/UtilsTests.swift
📚 Learning: 2025-10-13T21:33:37.731Z
Learnt from: CR
Repo: techouse/qs-swift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T21:33:37.731Z
Learning: Applies to Tests/QsSwiftTests/**/*.swift : Add unit tests in QsSwiftTests for new Swift options/behaviors
Applied to files:
Tests/QsObjCTests/ObjCDecodeTests.swiftSources/QsSwift/Internal/Decoder+ParseQuery.swiftTests/QsSwiftTests/DecodeTests.swiftTests/QsSwiftTests/UtilsTests.swift
📚 Learning: 2025-10-13T21:33:37.731Z
Learnt from: CR
Repo: techouse/qs-swift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T21:33:37.731Z
Learning: Applies to Tests/QsObjCTests/**/*.{m,mm,swift} : In ObjC tests, do not rely on NSDictionary enumeration order; explicitly sort when checking key order
Applied to files:
Tests/QsObjCTests/ObjCDecodeTests.swiftTests/QsSwiftTests/DecodeTests.swiftTests/QsSwiftTests/UtilsTests.swift
📚 Learning: 2025-10-13T21:33:37.731Z
Learnt from: CR
Repo: techouse/qs-swift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T21:33:37.731Z
Learning: Applies to Sources/QsObjC/**/*.{swift,h,m,mm} : Mirror new Swift options in ObjC bridge classes (QsEncodeOptions, QsDecodeOptions) only when applicable to ObjC users
Applied to files:
Tests/QsObjCTests/ObjCDecodeTests.swiftSources/QsSwift/Internal/Utils.swiftSources/QsSwift/Internal/Decoder+ParseQuery.swiftSources/QsSwift/Internal/Utils+Merge.swiftSources/QsSwift/Qs+Decode.swiftSources/QsSwift/Internal/Decoder+ParseObject.swift
📚 Learning: 2025-10-13T21:33:37.731Z
Learnt from: CR
Repo: techouse/qs-swift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T21:33:37.731Z
Learning: Applies to Sources/QsSwift/Internal/**/*.swift : For large indices above listLimit, convert arrays to maps and keep logic consistent
Applied to files:
Tests/QsObjCTests/ObjCDecodeTests.swiftSources/QsSwift/Internal/Utils.swiftSources/QsSwift/Internal/Decoder+ParseQuery.swiftSources/QsSwift/Internal/Utils+Overflow.swiftSources/QsSwift/Internal/Utils+Merge.swiftTests/QsSwiftTests/DecodeTests.swiftTests/QsSwiftTests/UtilsTests.swiftSources/QsSwift/Qs+Decode.swiftSources/QsSwift/Internal/Decoder+ParseObject.swiftSources/QsSwift/Internal/Utils+Combine.swift
📚 Learning: 2025-10-13T21:33:37.731Z
Learnt from: CR
Repo: techouse/qs-swift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T21:33:37.731Z
Learning: Applies to Sources/QsObjC/**/*.{swift,h,m,mm} : Keep Objective-C bridge wrappers thin, translating NSDictionary/NSArray/blocks to Swift equivalents without extra allocations
Applied to files:
Tests/QsObjCTests/ObjCDecodeTests.swiftSources/QsSwift/Internal/Utils.swiftSources/QsSwift/Internal/Utils+Merge.swiftTests/QsSwiftTests/DecodeTests.swiftSources/QsSwift/Qs+Decode.swiftSources/QsSwift/Internal/Decoder+ParseObject.swift
📚 Learning: 2025-10-13T21:33:37.731Z
Learnt from: CR
Repo: techouse/qs-swift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T21:33:37.731Z
Learning: Applies to Tests/**/*.{swift,m,mm} : Prefer explicit assertion helpers over broad equality when validating ordering or null semantics
Applied to files:
Tests/QsObjCTests/ObjCDecodeTests.swiftTests/QsSwiftTests/DecodeTests.swiftTests/QsSwiftTests/UtilsTests.swift
📚 Learning: 2025-10-13T21:33:37.731Z
Learnt from: CR
Repo: techouse/qs-swift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T21:33:37.731Z
Learning: Applies to Tests/QsSwiftTests/Fixtures/Data/EndToEndTestCases.swift : For end-to-end cases, only mutate EndToEndTestCases.swift and use OrderedDictionary for canonical shapes
Applied to files:
Tests/QsObjCTests/ObjCDecodeTests.swiftTests/QsSwiftTests/DecodeTests.swift
📚 Learning: 2025-10-13T21:33:37.731Z
Learnt from: CR
Repo: techouse/qs-swift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T21:33:37.731Z
Learning: Applies to Sources/QsSwift/**/*.swift : Respect null semantics: use NSNull() for null-like values and Undefined() to omit keys; honor strictNullHandling and skipNulls flags
Applied to files:
Sources/QsSwift/Internal/Utils.swiftSources/QsSwift/Internal/Decoder+ParseQuery.swiftSources/QsSwift/Internal/Utils+Merge.swiftTests/QsSwiftTests/DecodeTests.swiftSources/QsSwift/Qs+Decode.swiftSources/QsSwift/Internal/Decoder+ParseObject.swift
📚 Learning: 2025-10-13T21:33:37.731Z
Learnt from: CR
Repo: techouse/qs-swift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T21:33:37.731Z
Learning: Applies to Bench/**/*.{swift} : Keep changes allocation-neutral where possible; benchmark if touching tight loops
Applied to files:
Sources/QsSwift/Internal/Utils.swift
📚 Learning: 2025-10-13T21:33:37.731Z
Learnt from: CR
Repo: techouse/qs-swift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T21:33:37.731Z
Learning: Applies to Sources/QsSwift/Internal/**/*.swift : In hot paths (parsing/encoding engine), avoid unnecessary allocations and do not introduce new regex use unless already present
Applied to files:
Sources/QsSwift/Internal/Utils.swiftSources/QsSwift/Internal/Decoder+ParseQuery.swiftSources/QsSwift/Internal/Decoder+ParseObject.swift
📚 Learning: 2025-10-13T21:33:37.731Z
Learnt from: CR
Repo: techouse/qs-swift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T21:33:37.731Z
Learning: Applies to Sources/QsObjC/**/*.swift : In async helpers within the ObjC bridge, avoid captures that trigger Sendable warnings; keep closures minimal
Applied to files:
Sources/QsSwift/Internal/Utils.swift
📚 Learning: 2025-10-13T21:33:37.731Z
Learnt from: CR
Repo: techouse/qs-swift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T21:33:37.731Z
Learning: Applies to Sources/QsSwift/Internal/**/*.swift : Depth and parameter limits must remain O(length(query)); avoid quadratic regressions
Applied to files:
Sources/QsSwift/Internal/Utils.swiftSources/QsSwift/Internal/Decoder+ParseQuery.swiftTests/QsSwiftTests/DecodeTests.swift
📚 Learning: 2025-10-13T21:33:37.731Z
Learnt from: CR
Repo: techouse/qs-swift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T21:33:37.731Z
Learning: Applies to Tests/**/*.swift : Preserve deterministic ordering in fixtures and tests by using OrderedDictionary; do not replace with [String: Any] where ordering matters
Applied to files:
Sources/QsSwift/Internal/Utils.swiftTests/QsSwiftTests/DecodeTests.swiftTests/QsSwiftTests/UtilsTests.swift
📚 Learning: 2025-10-13T21:33:37.731Z
Learnt from: CR
Repo: techouse/qs-swift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T21:33:37.731Z
Learning: Applies to Sources/QsSwift/Qs.swift : When adding a new option or behavior, update the Swift option structs (EncodeOptions, DecodeOptions) in the public façade
Applied to files:
Sources/QsSwift/Internal/Utils.swiftSources/QsSwift/Internal/Decoder+ParseQuery.swiftSources/QsSwift/Internal/Utils+Overflow.swiftSources/QsSwift/Internal/Utils+Merge.swiftTests/QsSwiftTests/DecodeTests.swiftSources/QsSwift/Qs+Decode.swiftSources/QsSwift/Internal/Decoder+ParseObject.swift
📚 Learning: 2025-10-13T21:33:37.731Z
Learnt from: CR
Repo: techouse/qs-swift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T21:33:37.731Z
Learning: Applies to Sources/QsSwift/**/*.swift : Do not swallow errors in core encode/decode; propagate as thrown Swift errors
Applied to files:
Sources/QsSwift/Internal/Decoder+ParseQuery.swiftTests/QsSwiftTests/DecodeTests.swiftSources/QsSwift/Qs+Decode.swiftSources/QsSwift/Internal/Decoder+ParseObject.swift
📚 Learning: 2025-10-13T21:33:37.731Z
Learnt from: CR
Repo: techouse/qs-swift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T21:33:37.731Z
Learning: Applies to Sources/QsObjC/**/*.{swift,h,m,mm} : Maintain existing ObjC error domains and codes when introducing new failure modes
Applied to files:
Sources/QsSwift/Internal/Utils+Merge.swiftSources/QsSwift/Qs+Decode.swift
📚 Learning: 2025-10-13T21:33:37.731Z
Learnt from: CR
Repo: techouse/qs-swift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T21:33:37.731Z
Learning: Applies to Sources/QsObjC/**/*.{swift,h,m,mm} : Bridge null semantics correctly to ObjC, returning/accepting NSError where appropriate and preserving NSNull vs omitted key behavior
Applied to files:
Sources/QsSwift/Internal/Utils+Merge.swiftTests/QsSwiftTests/DecodeTests.swiftSources/QsSwift/Qs+Decode.swiftSources/QsSwift/Internal/Decoder+ParseObject.swift
📚 Learning: 2025-10-13T21:33:37.731Z
Learnt from: CR
Repo: techouse/qs-swift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T21:33:37.731Z
Learning: Applies to Sources/QsObjC/**/*.{swift,h,m,mm} : Map Objective-C blocks to Swift closures by unwrapping and passing through without extra allocations
Applied to files:
Sources/QsSwift/Qs+Decode.swiftSources/QsSwift/Internal/Decoder+ParseObject.swift
📚 Learning: 2025-10-13T21:33:37.731Z
Learnt from: CR
Repo: techouse/qs-swift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T21:33:37.731Z
Learning: Applies to Sources/QsSwift/Internal/**/*.swift : If sort closure is nil and encode=false, retain traversal order; maintain stability when adding traversal logic
Applied to files:
Sources/QsSwift/Internal/Decoder+ParseObject.swift
🧬 Code graph analysis (6)
Sources/QsSwift/Internal/Utils.swift (1)
Sources/QsSwift/Internal/Utils+Overflow.swift (1)
isOverflowKey(35-38)
Sources/QsSwift/Internal/Utils+Merge.swift (1)
Sources/QsSwift/Internal/Utils+Overflow.swift (5)
isOverflowKey(35-38)isOverflow(9-13)setOverflowMaxIndex(20-23)overflowMaxIndex(15-18)markOverflow(25-33)
Tests/QsSwiftTests/DecodeTests.swift (3)
Sources/QsSwift/Qs+Decode.swift (1)
decode(34-39)Sources/QsSwift/Internal/Utils+Decode.swift (1)
decode(12-39)Sources/QsSwift/Models/DecodeOptions.swift (1)
decode(243-254)
Sources/QsSwift/Qs+Decode.swift (2)
Sources/QsSwift/Internal/Utils+Overflow.swift (2)
isOverflowKey(35-38)isOverflow(9-13)Sources/QsSwift/Internal/Utils+Merge.swift (1)
merge(15-321)
Sources/QsSwift/Internal/Decoder+ParseObject.swift (2)
Sources/QsSwift/Internal/Utils+Overflow.swift (1)
isOverflow(9-13)Sources/QsSwift/Internal/Utils+Combine.swift (2)
combine(11-28)combine(32-47)
Sources/QsSwift/Internal/Utils+Combine.swift (1)
Sources/QsSwift/Internal/Utils+Overflow.swift (4)
isOverflow(9-13)markOverflow(25-33)overflowMaxIndex(15-18)setOverflowMaxIndex(20-23)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Swift tests (macos-15, Xcode 16.3, Swift 6.1)
- GitHub Check: Swift tests (Ubuntu, Swift 6.2 — experimental)
- GitHub Check: Swift tests (Ubuntu, Swift 6.1)
- GitHub Check: Swift tests (macos-26, Xcode 26, Swift 6.2 — experimental)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Analyze (swift)
🔇 Additional comments (10)
Sources/QsSwift/Internal/Utils.swift (1)
106-106: LGTM! Overflow key filtering is correctly applied to both dictionary iteration paths.The guards appropriately skip overflow metadata keys during deep bridging, ensuring they don't appear in the final
[String: Any]output. This is consistent with the overflow tracking mechanism inUtils+Overflow.swift.Also applies to: 117-117
Tests/QsObjCTests/ObjCDecodeTests.swift (1)
207-213: Good test coverage for overflow behavior with[]notation.The test correctly verifies that when
listLimit = 0, array notation falls back to dictionary representation with string keys. The assertions appropriately access keys individually rather than relying on enumeration order, which aligns with coding guidelines for ObjC tests.Sources/QsSwift/Internal/Decoder+ParseQuery.swift (1)
202-209: Correct integration of listLimit-aware combine with proper type handling.The cascade correctly handles all return types from the new
Utils.combine(_:_:listLimit:)overload:
[Any?]arrays are normalized by mappingniltoNSNull[Any]arrays are assigned directly- Overflow dictionaries (the else branch) are assigned as-is
The type check order is correct since
[Any?]and[Any]are distinct types in Swift.Sources/QsSwift/Internal/Decoder+ParseObject.swift (1)
58-70: Well-structured overflow-aware handling for[]root segments.The implementation correctly:
- Preserves already-overflow structures without re-wrapping (line 58-59)
- Maintains existing
allowEmptyListsbehavior (line 60-63)- Uses a unified combine-based approach that respects
listLimit(lines 65-70)The
valueForCombineclosure handles all leaf types appropriately before passing toUtils.combine, which will return either an array or overflow dictionary based on the limit. This aligns with the coding guideline to convert arrays to maps for large indices above listLimit.Sources/QsSwift/Qs+Decode.swift (3)
213-224: Well-implementedobjectifyOverflowhelper for normalizing overflow dictionaries.The closure efficiently converts overflow
[AnyHashable: Any]dictionaries to[String: Any]by:
- Using
reserveCapacityto minimize reallocations- Filtering overflow keys via
Utils.isOverflowKey- Properly converting
Intkeys to strings (preserving numeric indices)This ensures overflow structures are converted to a standard dictionary format before being used in the result.
239-244: Correct first-parsed overflow handling as a fast path.This branch appropriately handles the case where the first parsed value is an overflow dictionary, converting it via
objectifyOverflowbefore setting it as the initialobj. This mirrors the existing fast path for[String: Any]maps on lines 235-238.
273-279: Consistent overflow handling during the merge phase.The overflow branch correctly objectifies the overflow dictionary before merging, maintaining consistency with the list-to-indexed-map conversion pattern above it (lines 261-272). The merge result is properly cast and assigned.
Sources/QsSwift/Internal/Utils+Overflow.swift (1)
9-13: LGTM: overflow marker is minimal and avoids extra allocations.The sentinel-key approach (
OverflowKey+maxIndex) is lightweight and should be easy to strip during public “objectify” phases.Also applies to: 15-23, 25-33
Tests/QsSwiftTests/UtilsTests.swift (1)
780-865: Nice coverage for listLimit overflow + propagation (and avoids order traps).The tests validate both boundary conditions and that overflow metadata doesn’t interfere with consuming element values (filtering out overflow key before asserts).
Tests/QsSwiftTests/DecodeTests.swift (1)
558-562: LGTM: test expectations match the new “listLimit=0 yields map-like output” contract.Switching to
asDictStringaccess and asserting specific numeric-string keys keeps these tests deterministic and aligned with the new overflow/objectification behavior. As per coding guidelines, this avoids relying on dictionary enumeration order.Also applies to: 573-577, 936-963, 2240-2297, 2510-2527, 2925-2928
…objects and update overflow max index handling
…for overflow max index
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Sources/QsSwift/Internal/Utils+Overflow.swift (1)
18-21: InconsistentoverflowKeywrapping may cause lookup failures.Line 20 wraps
overflowKeyinAnyHashable(overflowKey)for lookup, but lines 30 and 39 usedict[overflowKey]directly. SinceOverflowKeyisHashable, both patterns should work due to Swift's dictionary subscript behavior, but consistency would be clearer.More importantly,
isOverflowKey(line 45) allocates a newAnyHashableon every call. Since this is called in hot paths (e.g., filtering overflow keys during merge), consider caching theAnyHashablewrapper or using a direct type check.♻️ Suggested optimization for hot-path usage
@usableFromInline - internal static let overflowKey = OverflowKey() + internal static let overflowKey = OverflowKey() + + @usableFromInline + internal static let overflowKeyHashable = AnyHashable(overflowKey) @inline(__always) @usableFromInline internal static func isOverflowKey(_ key: AnyHashable) -> Bool { - key == AnyHashable(overflowKey) + key == overflowKeyHashable }Also applies to: 28-31, 43-46
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
Sources/QsSwift/Internal/Utils+Combine.swiftSources/QsSwift/Internal/Utils+Merge.swiftSources/QsSwift/Internal/Utils+Overflow.swiftTests/QsSwiftTests/UtilsTests.swift
🚧 Files skipped from review as they are similar to previous changes (1)
- Sources/QsSwift/Internal/Utils+Combine.swift
🧰 Additional context used
📓 Path-based instructions (7)
Sources/QsSwift/Internal/**/*.swift
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Sources/QsSwift/Internal/**/*.swift: In hot paths (parsing/encoding engine), avoid unnecessary allocations and do not introduce new regex use unless already present
If sort closure is nil and encode=false, retain traversal order; maintain stability when adding traversal logic
For large indices above listLimit, convert arrays to maps and keep logic consistent
Depth and parameter limits must remain O(length(query)); avoid quadratic regressions
Files:
Sources/QsSwift/Internal/Utils+Merge.swiftSources/QsSwift/Internal/Utils+Overflow.swift
Sources/QsSwift/**/*.swift
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Sources/QsSwift/**/*.swift: Respect null semantics: use NSNull() for null-like values and Undefined() to omit keys; honor strictNullHandling and skipNulls flags
Do not swallow errors in core encode/decode; propagate as thrown Swift errors
Files:
Sources/QsSwift/Internal/Utils+Merge.swiftSources/QsSwift/Internal/Utils+Overflow.swift
**/*.{swift,h,m,mm}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use 4-space indentation across Swift and Objective-C sources
Files:
Sources/QsSwift/Internal/Utils+Merge.swiftTests/QsSwiftTests/UtilsTests.swiftSources/QsSwift/Internal/Utils+Overflow.swift
**/*.swift
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Doc comments should be concise and note option interactions (e.g., decodeDotInKeys implies allowDots)
Files:
Sources/QsSwift/Internal/Utils+Merge.swiftTests/QsSwiftTests/UtilsTests.swiftSources/QsSwift/Internal/Utils+Overflow.swift
Tests/**/*.swift
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Preserve deterministic ordering in fixtures and tests by using OrderedDictionary; do not replace with [String: Any] where ordering matters
Files:
Tests/QsSwiftTests/UtilsTests.swift
Tests/QsSwiftTests/**/*.swift
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Add unit tests in QsSwiftTests for new Swift options/behaviors
Files:
Tests/QsSwiftTests/UtilsTests.swift
Tests/**/*.{swift,m,mm}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Prefer explicit assertion helpers over broad equality when validating ordering or null semantics
Files:
Tests/QsSwiftTests/UtilsTests.swift
🧠 Learnings (18)
📓 Common learnings
Learnt from: CR
Repo: techouse/qs-swift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T21:33:37.731Z
Learning: Applies to Sources/QsSwift/Internal/**/*.swift : For large indices above listLimit, convert arrays to maps and keep logic consistent
📚 Learning: 2025-10-13T21:33:37.731Z
Learnt from: CR
Repo: techouse/qs-swift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T21:33:37.731Z
Learning: Applies to Sources/QsSwift/Internal/**/*.swift : For large indices above listLimit, convert arrays to maps and keep logic consistent
Applied to files:
Sources/QsSwift/Internal/Utils+Merge.swiftTests/QsSwiftTests/UtilsTests.swiftSources/QsSwift/Internal/Utils+Overflow.swift
📚 Learning: 2025-10-13T21:33:37.731Z
Learnt from: CR
Repo: techouse/qs-swift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T21:33:37.731Z
Learning: Applies to Sources/QsObjC/**/*.{swift,h,m,mm} : Keep Objective-C bridge wrappers thin, translating NSDictionary/NSArray/blocks to Swift equivalents without extra allocations
Applied to files:
Sources/QsSwift/Internal/Utils+Merge.swiftSources/QsSwift/Internal/Utils+Overflow.swift
📚 Learning: 2025-10-13T21:33:37.731Z
Learnt from: CR
Repo: techouse/qs-swift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T21:33:37.731Z
Learning: Applies to Sources/QsSwift/Qs.swift : When adding a new option or behavior, update the Swift option structs (EncodeOptions, DecodeOptions) in the public façade
Applied to files:
Sources/QsSwift/Internal/Utils+Merge.swiftSources/QsSwift/Internal/Utils+Overflow.swift
📚 Learning: 2025-10-13T21:33:37.731Z
Learnt from: CR
Repo: techouse/qs-swift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T21:33:37.731Z
Learning: Applies to Sources/QsObjC/**/*.{swift,h,m,mm} : Mirror new Swift options in ObjC bridge classes (QsEncodeOptions, QsDecodeOptions) only when applicable to ObjC users
Applied to files:
Sources/QsSwift/Internal/Utils+Merge.swift
📚 Learning: 2025-10-13T21:33:37.731Z
Learnt from: CR
Repo: techouse/qs-swift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T21:33:37.731Z
Learning: Applies to Sources/QsSwift/**/*.swift : Respect null semantics: use NSNull() for null-like values and Undefined() to omit keys; honor strictNullHandling and skipNulls flags
Applied to files:
Sources/QsSwift/Internal/Utils+Merge.swift
📚 Learning: 2025-10-13T21:33:37.731Z
Learnt from: CR
Repo: techouse/qs-swift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T21:33:37.731Z
Learning: Applies to Sources/QsObjC/**/*.{swift,h,m,mm} : Maintain existing ObjC error domains and codes when introducing new failure modes
Applied to files:
Sources/QsSwift/Internal/Utils+Merge.swift
📚 Learning: 2025-10-13T21:33:37.731Z
Learnt from: CR
Repo: techouse/qs-swift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T21:33:37.731Z
Learning: Applies to Sources/QsSwift/Internal/**/*.swift : In hot paths (parsing/encoding engine), avoid unnecessary allocations and do not introduce new regex use unless already present
Applied to files:
Sources/QsSwift/Internal/Utils+Merge.swift
📚 Learning: 2025-10-13T21:33:37.731Z
Learnt from: CR
Repo: techouse/qs-swift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T21:33:37.731Z
Learning: Applies to Sources/QsObjC/**/*.{swift,h,m,mm} : Bridge null semantics correctly to ObjC, returning/accepting NSError where appropriate and preserving NSNull vs omitted key behavior
Applied to files:
Sources/QsSwift/Internal/Utils+Merge.swiftSources/QsSwift/Internal/Utils+Overflow.swift
📚 Learning: 2025-10-13T21:33:37.731Z
Learnt from: CR
Repo: techouse/qs-swift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T21:33:37.731Z
Learning: Applies to Sources/QsObjC/**/*.{h,m,mm} : When exposing new enums or option flags to ObjC, keep naming consistent with existing cases (e.g., QsListFormatIndices)
Applied to files:
Sources/QsSwift/Internal/Utils+Merge.swift
📚 Learning: 2025-10-13T21:33:37.731Z
Learnt from: CR
Repo: techouse/qs-swift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T21:33:37.731Z
Learning: Applies to Sources/QsObjC/**/*.swift : In async helpers within the ObjC bridge, avoid captures that trigger Sendable warnings; keep closures minimal
Applied to files:
Sources/QsSwift/Internal/Utils+Merge.swift
📚 Learning: 2025-10-13T21:33:37.731Z
Learnt from: CR
Repo: techouse/qs-swift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T21:33:37.731Z
Learning: Applies to Tests/QsObjCTests/**/*.{m,mm,swift} : In ObjC tests, do not rely on NSDictionary enumeration order; explicitly sort when checking key order
Applied to files:
Sources/QsSwift/Internal/Utils+Merge.swiftTests/QsSwiftTests/UtilsTests.swift
📚 Learning: 2025-10-13T21:33:37.731Z
Learnt from: CR
Repo: techouse/qs-swift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T21:33:37.731Z
Learning: Applies to Tests/QsSwiftTests/**/*.swift : Add unit tests in QsSwiftTests for new Swift options/behaviors
Applied to files:
Tests/QsSwiftTests/UtilsTests.swift
📚 Learning: 2025-10-13T21:33:37.731Z
Learnt from: CR
Repo: techouse/qs-swift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T21:33:37.731Z
Learning: Applies to Tests/QsObjCTests/**/*.{m,mm,swift} : Add unit tests in QsObjCTests when features are bridged to ObjC
Applied to files:
Tests/QsSwiftTests/UtilsTests.swift
📚 Learning: 2025-10-13T21:33:37.731Z
Learnt from: CR
Repo: techouse/qs-swift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T21:33:37.731Z
Learning: Applies to Tests/**/*.{swift,m,mm} : Prefer explicit assertion helpers over broad equality when validating ordering or null semantics
Applied to files:
Tests/QsSwiftTests/UtilsTests.swift
📚 Learning: 2025-10-13T21:33:37.731Z
Learnt from: CR
Repo: techouse/qs-swift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T21:33:37.731Z
Learning: Applies to Tests/**/*.swift : Preserve deterministic ordering in fixtures and tests by using OrderedDictionary; do not replace with [String: Any] where ordering matters
Applied to files:
Tests/QsSwiftTests/UtilsTests.swift
📚 Learning: 2025-10-13T21:33:37.731Z
Learnt from: CR
Repo: techouse/qs-swift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T21:33:37.731Z
Learning: Applies to Tests/QsSwiftTests/Fixtures/Data/EndToEndTestCases.swift : For end-to-end cases, only mutate EndToEndTestCases.swift and use OrderedDictionary for canonical shapes
Applied to files:
Tests/QsSwiftTests/UtilsTests.swift
📚 Learning: 2025-10-13T21:33:37.731Z
Learnt from: CR
Repo: techouse/qs-swift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T21:33:37.731Z
Learning: Applies to Bench/**/*.{swift} : Keep changes allocation-neutral where possible; benchmark if touching tight loops
Applied to files:
Sources/QsSwift/Internal/Utils+Overflow.swift
🧬 Code graph analysis (1)
Sources/QsSwift/Internal/Utils+Merge.swift (1)
Sources/QsSwift/Internal/Utils+Overflow.swift (4)
isOverflow(17-21)overflowMaxIndex(23-26)setOverflowMaxIndex(28-31)markOverflow(33-41)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Swift tests (macos-26, Xcode 26, Swift 6.2 — experimental)
- GitHub Check: Swift tests (macos-15, Xcode 16.3, Swift 6.1)
- GitHub Check: Swift tests (Ubuntu, Swift 6.1)
- GitHub Check: Swift tests (Ubuntu, Swift 6.2 — experimental)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Analyze (swift)
🔇 Additional comments (13)
Sources/QsSwift/Internal/Utils+Overflow.swift (1)
1-58: Well-structured overflow utilities.The overflow infrastructure is cleanly designed with appropriate visibility modifiers and inline hints. The
OverflowKeymarker type prevents collision with user keys.Tests/QsSwiftTests/UtilsTests.swift (6)
780-805: Good boundary condition coverage forlistLimit.The test covers under-limit, exact-limit, over-limit, and zero-limit cases. The assertions verify both the overflow flag and the dictionary contents at expected indices.
807-821: Validates append-to-overflow semantics.The test correctly verifies that combining a scalar with an existing overflow object appends at the next available index (maxIndex + 1).
823-845: Comprehensive test for array flattening and null semantics.The test validates that arrays are flattened when appended to overflow objects, and correctly verifies that
nilelements are converted toNSNull. This aligns with the coding guidelines for null semantics.
847-869: Tests bidirectional overflow merging.The test validates merge behavior in both directions: merging a scalar into an overflow object and merging an overflow source into a primitive target. Both correctly preserve the overflow flag.
871-888: ValidatesmaxIndexpreservation through array merges.The test correctly verifies that when an overflow dictionary is merged into an array target, the overflow metadata (maxIndex) is preserved, allowing subsequent appends to use the correct index.
890-902: Thorough test for mixed key types.The test validates that
refreshOverflowMaxIndexcorrectly handles mixed key types (Int,NSNumber, andString), computing the max from only numeric keys while excluding the overflow marker key.Sources/QsSwift/Internal/Utils+Merge.swift (6)
19-42: Correct overflow propagation when merging array target with dict source.The logic correctly tracks
maxIndexfrom both the target array and source dictionary keys, propagates overflow metadata when the source is an overflow dictionary, and respects null semantics by convertingniltoNSNull().
44-60: Correct overflow extension when merging dict target with array source.When the target is an overflow dictionary, elements from the source array are correctly appended using incrementing indices starting from
maxIndex + 1, with proper null semantics.
163-180: Handles overflow dict target with arbitrary source types.The logic correctly extends overflow dictionaries with both sequence and scalar sources, filtering
Undefinedvalues and updating themaxIndexmetadata.
205-241: Complex but correct handling of overflow source with various target types.The logic correctly handles three cases:
- Array target: merges entries preserving both sets of indices
- Nil target: wraps as
NSNullat index 0, shifts source indices by 1- Scalar target: places at index 0, shifts source indices
The
newMax = sourceMax + 1calculation correctly accounts for the index shift.
288-320: Comprehensive overflow propagation in the main merge path.The logic correctly:
- Detects overflow status on both target and source
- Tracks the maximum numeric index during iteration
- Combines computed max with source's overflow metadata
- Sets overflow metadata on the result when either side is overflow
This ensures overflow semantics are preserved through recursive merges.
1-335: Overflow-aware merge logic is well-integrated.The merge function correctly handles all combinations of overflow and non-overflow targets/sources, preserving metadata through the merge hierarchy. The implementation aligns with the learning that "for large indices above listLimit, convert arrays to maps and keep logic consistent."
…Hashable wrapping
…kage@swift-5.10.swift
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Sources/QsSwift/Internal/Utils+Overflow.swift (1)
48-57: Consider adding a brief doc comment for the -1 sentinel semantics.The function sets
maxIndex = -1when no integer keys exist, which is a valid sentinel but could benefit from a short inline comment clarifying this represents "no valid indices found."📝 Suggested documentation
@usableFromInline + /// Scans non-overflow keys to compute the maximum integer index and stores it. + /// Sets -1 if no integer keys are present. internal static func refreshOverflowMaxIndex(_ dict: inout [AnyHashable: Any]) { var maxIndex = -1 for key in dict.keys where !isOverflowKey(key) {
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
Package.resolvedPackage.swiftPackage@swift-5.10.swiftSources/QsSwift/Internal/Utils+Overflow.swift
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{swift,h,m,mm}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use 4-space indentation across Swift and Objective-C sources
Files:
Package@swift-5.10.swiftSources/QsSwift/Internal/Utils+Overflow.swiftPackage.swift
**/*.swift
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Doc comments should be concise and note option interactions (e.g., decodeDotInKeys implies allowDots)
Files:
Package@swift-5.10.swiftSources/QsSwift/Internal/Utils+Overflow.swiftPackage.swift
Sources/QsSwift/Internal/**/*.swift
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Sources/QsSwift/Internal/**/*.swift: In hot paths (parsing/encoding engine), avoid unnecessary allocations and do not introduce new regex use unless already present
If sort closure is nil and encode=false, retain traversal order; maintain stability when adding traversal logic
For large indices above listLimit, convert arrays to maps and keep logic consistent
Depth and parameter limits must remain O(length(query)); avoid quadratic regressions
Files:
Sources/QsSwift/Internal/Utils+Overflow.swift
Sources/QsSwift/**/*.swift
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Sources/QsSwift/**/*.swift: Respect null semantics: use NSNull() for null-like values and Undefined() to omit keys; honor strictNullHandling and skipNulls flags
Do not swallow errors in core encode/decode; propagate as thrown Swift errors
Files:
Sources/QsSwift/Internal/Utils+Overflow.swift
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
Repo: techouse/qs-swift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T21:33:37.731Z
Learning: Applies to Sources/QsSwift/Internal/**/*.swift : For large indices above listLimit, convert arrays to maps and keep logic consistent
📚 Learning: 2025-10-13T21:33:37.731Z
Learnt from: CR
Repo: techouse/qs-swift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T21:33:37.731Z
Learning: Applies to Sources/QsSwift/Internal/**/*.swift : For large indices above listLimit, convert arrays to maps and keep logic consistent
Applied to files:
Sources/QsSwift/Internal/Utils+Overflow.swift
📚 Learning: 2025-10-13T21:33:37.731Z
Learnt from: CR
Repo: techouse/qs-swift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T21:33:37.731Z
Learning: Applies to Sources/QsSwift/Qs.swift : When adding a new option or behavior, update the Swift option structs (EncodeOptions, DecodeOptions) in the public façade
Applied to files:
Sources/QsSwift/Internal/Utils+Overflow.swift
📚 Learning: 2025-10-13T21:33:37.731Z
Learnt from: CR
Repo: techouse/qs-swift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T21:33:37.731Z
Learning: Applies to Sources/QsObjC/**/*.{swift,h,m,mm} : Keep Objective-C bridge wrappers thin, translating NSDictionary/NSArray/blocks to Swift equivalents without extra allocations
Applied to files:
Sources/QsSwift/Internal/Utils+Overflow.swift
📚 Learning: 2025-10-13T21:33:37.731Z
Learnt from: CR
Repo: techouse/qs-swift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T21:33:37.731Z
Learning: Applies to Bench/**/*.{swift} : Keep changes allocation-neutral where possible; benchmark if touching tight loops
Applied to files:
Sources/QsSwift/Internal/Utils+Overflow.swift
📚 Learning: 2025-10-13T21:33:37.731Z
Learnt from: CR
Repo: techouse/qs-swift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T21:33:37.731Z
Learning: Applies to Sources/QsObjC/**/*.{swift,h,m,mm} : Bridge null semantics correctly to ObjC, returning/accepting NSError where appropriate and preserving NSNull vs omitted key behavior
Applied to files:
Sources/QsSwift/Internal/Utils+Overflow.swift
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Swift tests (macos-26, Xcode 26, Swift 6.2 — experimental)
- GitHub Check: Swift tests (macos-15, Xcode 16.3, Swift 6.1)
- GitHub Check: Swift tests (Ubuntu, Swift 6.2 — experimental)
- GitHub Check: Swift tests (Ubuntu, Swift 6.1)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Analyze (swift)
🔇 Additional comments (7)
Sources/QsSwift/Internal/Utils+Overflow.swift (4)
3-7: LGTM!Good use of an empty struct as a sentinel key type. This ensures no collision with user-provided keys (strings, integers, etc.) while remaining
HashableandSendablefor thread-safe dictionary operations.
9-15: LGTM!The check order (native
Intfirst, thenNSNumberfallback) is correct. Note thatNSNumber.intValuetruncates floats and converts bools, which is acceptable behavior for array index extraction in this context.
17-31: LGTM!The detection and accessor functions are concise and efficient. The
inoutparameter forsetOverflowMaxIndexcorrectly enables in-place mutation without extra allocations.
33-41: LGTM!The dictionary copy is necessary here for the immutable return semantics. Since this function is called only when overflow conditions are detected (not the common path), the allocation is justified.
Package.swift (1)
15-18: LGTM!The ReerKit version bump is consistent with
Package@swift-5.10.swift, ensuring both Swift toolchain versions resolve to the same dependency version on Linux.Package.resolved (1)
1-3: LGTM!The
originHashupdate reflects the dependency manifest changes. Note that ReerKit doesn't appear in the pins because it's conditionally included only on Linux (#if os(Linux)), and this resolved file was generated on a non-Linux platform.Package@swift-5.10.swift (1)
16-18: These are newly created package files, not a version update.Package@swift-5.10.swift and Package.swift were added in this commit as new files with ReerKit 1.2.2, not updated from a prior version. Both files correctly use 4-space indentation and maintain consistent Linux-only conditional dependency inclusion.
Likely an incorrect or invalid review comment.
This pull request introduces a new overflow object representation for decoded lists that exceed the configured
listLimit, ensuring correct handling and merging of large lists in query string decoding. It refactors internal utilities to support this overflow logic, updates decoding and merging behavior to recognize and process overflow objects, and adds comprehensive test coverage for these scenarios.Overflow object implementation and utilities
Utils+Overflow.swiftto define overflow object mechanics, including marking, checking, and managing the maximum index for overflowed lists. This enables lists that exceedlistLimitto be represented as a dictionary with integer keys and a special overflow marker.Utils+Combine.swiftandUtils+Merge.swift) to use the new overflow object format when combining or merging lists that exceed the limit, ensuring consistent overflow handling throughout decoding and merging. [1] [2] [3] [4] [5]Decoder and query parsing updates
Decoder+ParseObject.swiftandDecoder+ParseQuery.swiftto create and propagate overflow objects when decoding lists that exceedlistLimit, and to handle merging and normalization of overflow objects within parsed results. [1] [2]Decoded object conversion and integration
Qs+Decode.swiftto convert overflow objects into standard string-keyed dictionaries for final output, ensuring decoded results remain accessible and consistent for consumers, and to merge overflow objects correctly when combining multiple parsed fragments. [1] [2] [3]Test coverage for overflow behavior
listLimit, including conversion to overflow objects and correct merging behavior. [1] [2] [3] [4] [5]Miscellaneous
ReerKitdependency version for Linux in bothPackage.swiftandPackage@swift-5.10.swift. [1] [2]Relates to GHSA-6rw7-vpxm-498p
Summary by CodeRabbit
New Features
Improvements
Tests
✏️ Tip: You can customize this high-level summary in your review settings.