new-log-viewer: Add bounds checking and search methods to support log filtering around a specific log event. #81
Conversation
WalkthroughThe changes introduce new functions and modifications to the export statements in two files: Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DataUtils
participant SearchUtils
User->>DataUtils: Call isWithinBounds(data, x)
DataUtils-->>User: Return true/false
User->>DataUtils: Call clampWithinBounds(data, num)
DataUtils-->>User: Return clamped value
User->>SearchUtils: Call findLargestIdxLte(data, x)
SearchUtils->>SearchUtils: Perform binarySearch
SearchUtils-->>User: Return largest index i where data[i] <= x
User->>SearchUtils: Call strictFindLargestIdxLte(data, x)
SearchUtils->>SearchUtils: Check bounds
SearchUtils->>SearchUtils: Perform binarySearch
SearchUtils-->>User: Return largest index i or null
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (2)
new-log-viewer/src/utils/search.ts (2)
59-59: Unnecessary type assertion withas numberIn line 59, the type assertion
as numberis unnecessary becausedatais already an array of numbers. Removing the type assertion simplifies the code without affecting functionality.Suggested change:
- const firstGreaterIdx: number = binarySearch(n, (i) => data[i] as number > x); + const firstGreaterIdx: number = binarySearch(n, (i) => data[i] > x);
94-95: Unnecessary type assertion withas numberSimilar to the previous function, in line 94, the type assertion
as numberis unnecessary. Removing it enhances code readability.Suggested change:
const firstGreaterIdx: number = binarySearch(n, (i) => data[i] as number > x);Modify to:
const firstGreaterIdx: number = binarySearch(n, (i) => data[i] > x);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- new-log-viewer/src/utils/search.ts (1 hunks)
🔇 Additional comments (1)
new-log-viewer/src/utils/search.ts (1)
18-35: Well-implementedbinarySearchfunctionThe
binarySearchfunction is correctly implemented with clear documentation and example usage. It adheres to best practices and provides a generic utility for binary search operations.
new-log-viewer/src/utils/search.ts
Outdated
There was a problem hiding this comment.
Nit: shall we place those util functions into utils/data.ts?
There was a problem hiding this comment.
sure i'll move them
new-log-viewer/src/utils/search.ts
Outdated
| * const result = strictFindLargestIdxLte(arr, 8); | ||
| * console.log(result); // Output: 3 (since arr[3] is 7). | ||
| */ | ||
| const strictFindLargestIdxLte = (data: number[], x: number): Nullable<number> => { |
There was a problem hiding this comment.
I tried reading the PR description but can't understand the purpose of this function. Would you mind helping me to understand it a bit?
Say we are performing binary search in an array, it's with the assumption that the numbers are strictly incremental. If we want to make sure the desired number x is within the range of data[0] and data[data.length-1], why don't we do the comparisons before we kick off the binary search? i.e. Why are we not coming up with a withinRange(data: number[], x: number) function instead?
There was a problem hiding this comment.
Sure that might make more sense lol. I added a new functionisWithinBounds = (data: number[], x: number): boolean and removed strictFindLargestIdxLte
Co-authored-by: Junhao Liao <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
new-log-viewer/src/utils/search.ts (2)
4-35: LGTM: Well-implemented and documented binary search function.The
binarySearchfunction is correctly implemented and efficiently finds the smallest index where the condition function returns true. The documentation is comprehensive and includes a helpful example.Consider exporting this function alongside the others, as it could be useful in other parts of the application. This would promote code reuse and avoid duplication.
To export the function, modify the export statement at the end of the file:
export { + binarySearch, findLargestIdxLte, strictFindLargestIdxLte, };
108-111: LGTM: Exports are correct, consider adding binarySearch.The export statement correctly includes the main functions
findLargestIdxLteandstrictFindLargestIdxLte. As mentioned earlier, consider also exportingbinarySearchfor potential reuse in other parts of the application.To include
binarySearchin the exports, modify the export statement:export { + binarySearch, findLargestIdxLte, strictFindLargestIdxLte, };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- new-log-viewer/src/utils/search.ts (1 hunks)
🧰 Additional context used
🪛 GitHub Check: lint-check
new-log-viewer/src/utils/search.ts
[failure] 52-52:
'length' is assigned a value but never used
[failure] 59-59:
Unsafe argument of typeanyassigned to a parameter of typenumber
🔇 Additional comments (1)
new-log-viewer/src/utils/search.ts (1)
1-1: LGTM: Import statement is appropriate.The import of
Nullablefrom "../typings/common" is correct and necessary for the function signatures in this file.
There was a problem hiding this comment.
lgtm. @kirkrodrigues could you check if the docs make sense to you? Feel free to modify my suggestions.
new-log-viewer/src/utils/data.ts
Outdated
| * @param x Target value. | ||
| * @return True if is `x` is within bounds and false if outside of bounds or array is empty. | ||
| */ | ||
| const isWithinBounds = (data: number[], x: number): boolean => { |
There was a problem hiding this comment.
nit: personally I prefer range in the function name (i.e. isWithinRange()) over bounds as in we have methods named decode_range() or similar elsewhere lol. not a strong preference though
There was a problem hiding this comment.
I chose bounds to distinguish from range. Felt like range is used for the indexes elsewhere, but here we are referring to the values of the array. Let me know if you still prefer range. I don't have strong preference.
There was a problem hiding this comment.
gotcha. let's keep bounds then
new-log-viewer/src/utils/data.ts
Outdated
| * const result = findLargestIdxLte(arr, 8); | ||
| * console.log(result); // Output: 3 (since arr[3] is 7). | ||
| */ | ||
| const findLargestIdxLte = (data: number[], x: number): Nullable<number> => { |
There was a problem hiding this comment.
For consistency I personally prefer get (i.e., getLargestIdxLte) over find.
new-log-viewer/src/utils/data.ts
Outdated
|
|
||
| /** | ||
| * Finds the largest index `i` in a sorted array `data` such that `data[i] <= x`. | ||
| * Uses binary search for efficiency. Returns 0 if `x` is less than `data[0]`. |
There was a problem hiding this comment.
| * Uses binary search for efficiency. Returns 0 if `x` is less than `data[0]`. | |
| * Uses binary search for efficiency. |
There was a problem hiding this comment.
I prefer to leave this in. The comment is here as this is not normal behaviour for binary search. Would normally just return length for not found, but this is custom use case.
There was a problem hiding this comment.
Right, I got that the fallback value is worth documenting. I meant to say we want to follow this rule: "The description should be omitted if the return value description would repeat the description."
There was a problem hiding this comment.
I see. Okay makes i remove.
Co-authored-by: Junhao Liao <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
new-log-viewer/src/utils/data.ts (3)
20-52: LGTM! Well-implemented generic binary search function.The
binarySearchfunction is correctly implemented based on Go's standard library, which is a reliable source. The generic implementation allows for versatile use in various binary search scenarios.One minor suggestion to improve the example in the JSDoc:
Consider modifying the example to showcase the generic nature of the function:
/** * @example * const arr = [1, 3, 5, 7, 10, 15, 20]; * const result = binarySearch(arr.length, (i) => arr[i] >= 10); * console.log(result); // Output: 4 (since arr[4] is 10, the first element >= 10) * * // Example with a custom condition * const firstEven = binarySearch(arr.length, (i) => arr[i] % 2 === 0); * console.log(firstEven); // Output: 4 (since 10 is the first even number in the array) */This addition demonstrates the function's flexibility with different condition functions.
54-83: LGTM! Efficient implementation using binary search.The
getLargestIdxLtefunction is well-implemented, making efficient use of thebinarySearchfunction. It correctly handles all edge cases and is well-documented.One minor suggestion to improve the example in the JSDoc:
Consider modifying the example to showcase more scenarios:
/** * @example * const arr = [2, 3, 5, 7, 10, 15, 20]; * console.log(getLargestIdxLte(arr, 8)); // Output: 3 (since arr[3] is 7) * console.log(getLargestIdxLte(arr, 2)); // Output: 0 (since arr[0] is 2) * console.log(getLargestIdxLte(arr, 1)); // Output: 0 (edge case: x < arr[0]) * console.log(getLargestIdxLte(arr, 20)); // Output: 6 (since arr[6] is 20) * console.log(getLargestIdxLte([], 5)); // Output: null (edge case: empty array) */This addition demonstrates the function's behaviour in various scenarios, including edge cases.
122-127: Consider reordering exports alphabetically.The export statement has been correctly updated to include the new functions. However, for consistency and ease of maintenance, consider reordering the exports alphabetically.
Here's the suggested order:
export { getLargestIdxLte, getMapKeyByValue, getMapValueWithNearestLessThanOrEqualKey, isWithinBounds, };This alphabetical ordering makes it easier to locate specific exports, especially as the list grows.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- new-log-viewer/src/utils/data.ts (2 hunks)
🔇 Additional comments (1)
new-log-viewer/src/utils/data.ts (1)
4-18: LGTM! Function implementation and documentation are well-done.The
isWithinBoundsfunction is correctly implemented and well-documented. The logic handles the empty array case and correctly checks if the value is within bounds.Regarding the past comment about using 'range' instead of 'bounds' in the function name, I agree with the current choice of 'bounds'. It clearly distinguishes this function from others that might deal with index ranges, making the code more self-explanatory.
new-log-viewer/src/utils/data.ts
Outdated
| * @return The largest index where `data[i] <= target`. There are 2 edge cases where returns: | ||
| * - 0 if `target` is less than `data[0]`. | ||
| * - `null` if array is empty. |
There was a problem hiding this comment.
I find the 0 return case confusing:
If we're looking for i such that data[i] <= target, then
- if
targetis not in the bounds ofdata, we return 0. - if
data[0] <= target, we also return 0.
Since these two cases are indistinguishable, I feel like we should instead return length?
There was a problem hiding this comment.
Explained in person. Maybe we can call the function findNearestIndexLessThanOrEqual or findNearestIndex. I feel like nearest is more vague, so the edge case makes more sense.
There was a problem hiding this comment.
This is also sort of inline with getMapValueWithNearestLessThanOrEqualKey later in the file
There was a problem hiding this comment.
I think we can fix this by clamping the value by the bounds of the array before calling the function. I know you already rewrote the docs... Let me try this, and ill request your review
new-log-viewer/src/utils/data.ts
Outdated
| * const result = binarySearch(arr.length, (i) => arr[i] >= 10); | ||
| * console.log(result); // Output: 4 (since arr[4] is 10). | ||
| */ | ||
| const binarySearch = (length: number, conditionFn: (index: number) => boolean): number => { |
There was a problem hiding this comment.
Can we rename conditionFn to binaryPredicate? I think that's usually what it's called.
There was a problem hiding this comment.
I changed to predicate
new-log-viewer/src/utils/data.ts
Outdated
| * const result = getLargestIdxLte(arr, 8); | ||
| * console.log(result); // Output: 3 (since arr[3] is 7). | ||
| */ | ||
| const getLargestIdxLte = (data: number[], target: number): Nullable<number> => { |
There was a problem hiding this comment.
This name is a bit confusing to me. How about findLessThanOrEqualElement? (I know what Lte means but it could be misinterpreted as LTE, lol.)
There was a problem hiding this comment.
How about findNearestIndexLessThanOrEqual or findNearestIndex
There was a problem hiding this comment.
How about findFirstNearestElement?
- "first nearest" is an abstract concept so hopefully users of the function will read the docstring before using it.
- I think using "element" instead of "index" is a bit more intuitive since we really care about the element; it's just that the function indicates the found element by returning an index.
Co-authored-by: kirkrodrigues <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
new-log-viewer/src/utils/data.ts (3)
4-18: LGTM! Consider using non-null assertion for improved type safety.The implementation of
isWithinBoundslooks good. The function correctly handles the empty array case and efficiently checks if the target is within bounds.A minor suggestion for improvement:
Consider using non-null assertion operator (
!) instead of type assertion (as number) for improved type safety:- return (target >= (data[0] as number)) && (target <= (data[length - 1] as number)); + return (target >= data[0]!) && (target <= data[length - 1]!);This change assumes that
datais non-empty, which is guaranteed by the previous check.
20-48: LGTM! Consider renamingconditionFntobinaryPredicate.The implementation of
binarySearchis correct and efficient. The documentation is clear and the included example is helpful.A minor suggestion for improvement:
Consider renaming the
conditionFnparameter tobinaryPredicateas suggested in a previous review comment. This name is more commonly used in the context of binary search algorithms:-const binarySearch = (length: number, conditionFn: (index: number) => boolean): number => { +const binarySearch = (length: number, binaryPredicate: (index: number) => boolean): number => { // ... (update usage inside the function)This change would improve consistency with common terminology in computer science.
50-79: LGTM! Consider a more descriptive function name.The implementation of
getLargestIdxLteis correct and efficient. It properly handles edge cases and uses thebinarySearchfunction effectively.A suggestion for improvement:
Consider renaming the function to make its purpose more immediately clear. As suggested in a previous review comment, a name like
findLessThanOrEqualElementmight be more descriptive:-const getLargestIdxLte = (data: number[], target: number): Nullable<number> => { +const findLessThanOrEqualElement = (data: number[], target: number): Nullable<number> => { // ... (function body remains the same)This change would make the function's purpose more self-explanatory, potentially improving code readability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- new-log-viewer/src/utils/data.ts (2 hunks)
🔇 Additional comments (2)
new-log-viewer/src/utils/data.ts (2)
118-123: LGTM! Export statement is correct and well-organized.The export statement has been properly updated to include the new functions. The alphabetical ordering of the exported functions enhances readability and makes it easier to locate specific exports.
Line range hint
1-124: Overall, excellent implementation aligning with PR objectives.The new binary search methods introduced in this file provide a solid foundation for efficiently locating log events in a filtered log event map. The implementations are correct, well-documented, and include helpful examples. These changes align well with the PR objectives of supporting future log filtering capabilities.
The suggested minor improvements in naming and type assertions will further enhance the code's readability and type safety. Great job on this implementation!
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
new-log-viewer/src/utils/data.ts (3)
4-18: LGTM! Consider removing unnecessary type assertions.The function logic is correct, and the implementation addresses the previous review comments. However, you might want to consider removing the unnecessary type assertions
as numberif thedataarray is properly typed asnumber[].Here's a suggested improvement:
- return (target >= (data[0] as number)) && (target <= (data[length - 1] as number)); + return target >= data[0] && target <= data[length - 1];
20-48: LGTM! Consider using a more idiomatic comparison.The binary search implementation looks correct and efficient. The documentation is clear and includes a helpful example. Great job on making it generic and reusable!
For improved readability, consider changing the comparison in the
ifstatement to a more idiomatic form:- if (false === predicate(mid)) { + if (!predicate(mid)) {This change makes the code more concise and easier to read at a glance.
50-79: LGTM! Consider using a more descriptive function name.The implementation looks correct and efficient, making good use of the
binarySearchfunction. The documentation is clear and includes a helpful example.While the function name has been updated based on previous comments, consider using a more descriptive name that avoids abbreviations:
-const getLargestIdxLte = (data: number[], target: number): Nullable<number> => { +const getLargestIndexLessThanOrEqual = (data: number[], target: number): Nullable<number> => {This change makes the function name more self-explanatory, which can improve code readability and maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- new-log-viewer/src/utils/data.ts (2 hunks)
🔇 Additional comments (2)
new-log-viewer/src/utils/data.ts (2)
118-123: LGTM! Export statement is correct and well-organized.The export statement has been properly updated to include all the necessary functions, including the newly added ones. The alphabetical ordering of the exports is a nice touch for consistency and readability.
Line range hint
1-124: Overall, excellent implementation of binary search utilities.The new functions (
isWithinBounds,binarySearch, andgetLargestIdxLte) are well-implemented and documented. They align perfectly with the PR objectives of introducing binary search methods to support future log filtering. The code is efficient, readable, and includes helpful examples in the documentation.A few minor suggestions have been made to further improve code clarity and consistency:
- Remove unnecessary type assertions in
isWithinBounds.- Use a more idiomatic comparison in
binarySearch.- Consider a more descriptive name for
getLargestIdxLte.These changes are not critical but could enhance the overall quality of the code. Great job on implementing these utility functions!
| * Finds the largest index `i` in a sorted array `data` such that `data[i] <= target`. Uses binary | ||
| * search for efficiency. |
There was a problem hiding this comment.
| * Finds the largest index `i` in a sorted array `data` such that `data[i] <= target`. Uses binary | |
| * search for efficiency. | |
| * Finds the index `i` in a sorted array `data` such that `data[i]` is the "first nearest" element | |
| * to `target`, where "first nearest" is defined as follows: | |
| * - If `target` is in or greater than the range of `data`'s elements, then `i` is the largest index | |
| * such that `data[i] <= target`. | |
| * - Otherwise, `target` is less than the range of `data`'s elements, so `i` is 0 since `data[0]` is | |
| * the array element closest to `target`. |
There was a problem hiding this comment.
see review comment
new-log-viewer/src/utils/data.ts
Outdated
| * @return The largest index where `data[i] <= target` or: | ||
| * - 0 if `target` is less than `data[0]`. | ||
| * - `null` if array is empty. |
There was a problem hiding this comment.
| * @return The largest index where `data[i] <= target` or: | |
| * - 0 if `target` is less than `data[0]`. | |
| * - `null` if array is empty. | |
| * @return The index of the first nearest (as defined above) element or `null` if the array is | |
| * empty. |
There was a problem hiding this comment.
see review comment
new-log-viewer/src/utils/data.ts
Outdated
| const firstGreaterIdx: number = binarySearch(length, (i) => data[i] as number > target); | ||
|
|
||
| if (0 === firstGreaterIdx) { | ||
| return 0; |
There was a problem hiding this comment.
| return 0; | |
| // `target` < `data[0]` so the first nearest element is `data[0]`. | |
| return 0; |
There was a problem hiding this comment.
see review comment
new-log-viewer/src/utils/data.ts
Outdated
| * const result = getLargestIdxLte(arr, 8); | ||
| * console.log(result); // Output: 3 (since arr[3] is 7). | ||
| */ | ||
| const getLargestIdxLte = (data: number[], target: number): Nullable<number> => { |
There was a problem hiding this comment.
How about findFirstNearestElement?
- "first nearest" is an abstract concept so hopefully users of the function will read the docstring before using it.
- I think using "element" instead of "index" is a bit more intuitive since we really care about the element; it's just that the function indicates the found element by returning an index.
There was a problem hiding this comment.
I tried a different approach where the interface for findNearestLessThanOrEqualElement is closer to what you would expect for binary search. Now to use for our use case for searching the filtered log event map, first you call clampWithinBounds followed by findNearestLessThanOrEqualElement, this will achieve the same effect as the previous less intuitive single function.
| * Finds the largest index `i` in a sorted array `data` such that `data[i] <= target`. Uses binary | ||
| * search for efficiency. |
There was a problem hiding this comment.
see review comment
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
new-log-viewer/src/utils/data.ts (3)
5-19: LGTM with a minor suggestion.The function looks good and implements the required logic correctly. The documentation is clear and concise.
However, consider removing the type assertions
as numberif thedataparameter is properly typed asnumber[]. These assertions are unnecessary and may hide potential type errors.Here's a suggested improvement:
- return (target >= (data[0] as number)) && (target <= (data[length - 1] as number)); + return (target >= data[0]) && (target <= data[length - 1]);
21-35: LGTM with a minor suggestion.The function is well-implemented and properly documented. It correctly handles the edge case of an empty array and uses the
clamputility function, which is good for readability and maintainability.Similar to the
isWithinBoundsfunction, consider removing the type assertionsas numberif thedataparameter is properly typed asnumber[].Here's a suggested improvement:
- return clamp(num, data[0] as number, data[length - 1] as number); + return clamp(num, data[0], data[length - 1]);
67-96: LGTM with a minor suggestion.The function is well-implemented and properly documented. It correctly handles the edge case of an empty array and uses an efficient binary search algorithm. The function name and documentation are clear and have been approved in previous reviews.
As with the previous functions, consider removing the type assertion
as numberif thedataparameter is properly typed asnumber[].Here's a suggested improvement:
- const firstGreaterIdx: number = binarySearch(length, (i) => data[i] as number > target); + const firstGreaterIdx: number = binarySearch(length, (i) => data[i] > target);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- new-log-viewer/src/utils/data.ts (2 hunks)
🔇 Additional comments (2)
new-log-viewer/src/utils/data.ts (2)
135-141: LGTM! Export statement updated correctly.The export statement has been properly updated to include all the new functions while maintaining the existing exports. The alphabetical sorting of the exports is a nice touch for readability and maintainability.
Line range hint
1-141: Overall, excellent implementation aligning with PR objectives.The new utility functions (
isWithinBounds,clampWithinBounds, andfindNearestLessThanOrEqualElement) provide a solid foundation for implementing efficient log filtering capabilities in the y-scope log viewer. These functions, particularly the binary search implementation, will enable the back-end to efficiently locate log events in a filtered log event map, as outlined in the PR objectives.The implementation is well-documented and includes proper error handling for edge cases. The use of binary search ensures efficient performance, which is crucial for handling large log datasets.
While the functions are not yet invoked in the existing code, they are now properly exported and ready for use in implementing the log filtering feature. This sets the stage for future development of the log viewer's capabilities, particularly in supporting front-end requests for specific log event numbers.
davemarco
changed the title
References
new-log-viewer series: #45 #46 #48 #51 #52 #53 #54 #55 #56 #59 #60 #61 #62 #63 #66 #67 #68 #69 #70 #71 #72 #73 #74 #76
Description
The new event num cursor in #76 allows the front-end to request a page from the back-end using a log event number. When log events are filtered, the back-end must be able to search for that log event number in the filtered log event map. Specifically, the back-end response must return the closest existing log event in the map, and all the events on the page with the closest log event.
For example consider the following filtered log event map (log event num are values), and a user setting of 3 events per page (simplified for example).
[2, 6, 8, 22, 123, 186].
If the front-end requests log-event number 146. The back end should be able to find and return closest log event (123), as well as send back decoded events 22, 123, 186 (i.e. the second page).
This PR allows the back-end to find the index of the closest log event in the filtered map with binary search methods.
Where methods will be used
The new search utils file exports two similar methods. The implement binary search to find the largest less than or equal index ("the closest log event") and provide slightly different behaviour for edge cases.
findLargestIdxLteis useful for the example above, where we want to find the closest log event in the back-end. If the value requested is outside the range, for example, say the user requested event 10,000 in the example above, it would just return 186.strictFindLargestIdxLteis useful for the front-end to avoid unnecessary event num cursor requests. Say the user inputs a new log event number, this method can be used to find acloselog event on the page (front-end stores all the log events on the current page). If there is a close log event, it will return an index, and the back-end request is never sent. It is strict in that if the log event is not on the page, it will let the caller know by retuning null, and a request can be sent.Note this methods are just implemented and they are not called anywhere yet.
Validation performed
Tested basic functionality.
Summary by CodeRabbit