Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 7 additions & 25 deletions src/services/decoders/JsonlDecoder/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
LogLevelFilter,
} from "../../../typings/logs";
import {getNestedJsonValue} from "../../../utils/js";
import {upperBoundBinarySearch} from "../../../utils/math";
import YscopeFormatter from "../../formatters/YscopeFormatter";
import {parseFilterKeys} from "../utils";
import {
Expand Down Expand Up @@ -139,31 +140,12 @@ class JsonlDecoder implements Decoder {
}

findNearestLogEventByTimestamp (timestamp: number): Nullable<number> {
let low = 0;
let high = this.#logEvents.length - 1;
if (high < low) {
return null;
}

while (low <= high) {
const mid = Math.floor((low + high) / 2);

// `mid` is guaranteed to be within bounds since `low <= high`.
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const midTimestamp = this.#logEvents[mid]!.timestamp.valueOf();
if (midTimestamp <= timestamp) {
low = mid + 1;
} else {
high = mid - 1;
}
}

// corner case: all log events have timestamps >= timestamp
if (0 > high) {
return 0;
}

return high;
return upperBoundBinarySearch(
(idx) => this.#logEvents[idx]?.timestamp.valueOf(),
0,
this.#logEvents.length - 1,
timestamp
);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/typings/decoders.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ interface Decoder {
* behaviour.
*
* @param timestamp
* @return The index of the log event L.
* @return The index of the log event L, or null if there are no log events.
*/
findNearestLogEventByTimestamp(timestamp: number): Nullable<number>;
}
Expand Down
45 changes: 45 additions & 0 deletions src/utils/math.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
import {Nullable} from "../typings/common";


/**
* Clamps a number to the given range. E.g. If the number is greater than the range's upper bound,
* the range's upper bound is returned.
Expand All @@ -20,7 +23,49 @@ const clamp = (num: number, min: number, max: number) => Math.min(Math.max(num,
const getChunkNum =
(itemNum: number, chunkSize: number) => Math.max(1, Math.ceil(itemNum / chunkSize));

/**
* Finds the last element in a sorted collection that is less than or equal to `upperboundValue`. If
* all elements in the collection are greater than `upperboundValue`, returns the first index of the
* collection (i.e., `0`).
*
* @param get Function to access elements in the collection by index.
* @param lowIdx
* @param highIdx
* @param upperBoundValue
* @return The index of the last element less than or equal to `upperboundValue`, `0` if all
* elements are greater, or `null` if the collection is empty or the indices are invalid.
*/
const upperBoundBinarySearch = <T>(
get: (index: number) => T,
lowIdx: number,
highIdx: number,
upperBoundValue: T,
): Nullable<number> => {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Type-safety: constrain T or accept a comparator to ensure valid <= comparisons.

get(mid) <= upperBoundValue assumes T is comparable with <=. To make this safe and self-documenting, either constrain T to a comparable type (e.g., number) or accept a comparator.

Minimal change (matches current usage with timestamps as numbers):

-const upperBoundBinarySearch = <T>(
+const upperBoundBinarySearch = <T extends number>(
     get: (index: number) => T,
     lowIdx: number,
     highIdx: number,
     upperBoundValue: T,
 ): Nullable<number> => {

Alternatively (if you want generic support), introduce a comparator:

const upperBoundBinarySearchBy = <T>(
  get: (index: number) => T,
  lowIdx: number,
  highIdx: number,
  upperBoundValue: T,
  cmp: (a: T, b: T) => number,
): Nullable<number> => {
  if (highIdx < lowIdx || "undefined" === typeof get(highIdx)) return null;
  while (lowIdx <= highIdx) {
    const mid = Math.floor((lowIdx + highIdx) / 2);
    if (cmp(get(mid), upperBoundValue) <= 0) lowIdx = mid + 1;
    else highIdx = mid - 1;
  }
  return 0 > highIdx ? 0 : highIdx;
};
🤖 Prompt for AI Agents
In src/utils/math.ts around lines 38 to 43, the binary search uses get(mid) <=
upperBoundValue on a generic T which is unsafe; either constrain T to a
comparable type (e.g., change signature to <T extends number>) so numeric
comparisons are type-safe, or add a comparator argument (e.g., cmp: (a: T, b: T)
=> number), replace the <= check with cmp(get(mid), upperBoundValue) <= 0, and
update the return logic and all callers accordingly (or expose a new
upperBoundBinarySearchBy name if you want to keep the old API).

if (highIdx < lowIdx || "undefined" === typeof (get(highIdx))) {
return null;
}

while (lowIdx <= highIdx) {
const mid = Math.floor((lowIdx + highIdx) / 2);

// `mid` is guaranteed to be within bounds since `low <= high`.
if (get(mid) <= upperBoundValue) {
lowIdx = mid + 1;
} else {
highIdx = mid - 1;
}
}

// corner case: all values >= upperboundValue
if (0 > highIdx) {
return 0;
}

return highIdx;
};

export {
clamp,
getChunkNum,
upperBoundBinarySearch,
};
52 changes: 52 additions & 0 deletions test/utils/math.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
/* eslint-disable @stylistic/array-element-newline */
import {
clamp,
getChunkNum,
upperBoundBinarySearch,
} from "../../src/utils/math";


Expand Down Expand Up @@ -49,3 +51,53 @@ describe("getChunkNum", () => {
expect(getChunkNum(20, 4)).toBe(5);
});
});

describe("upperBoundBinarySearch", () => {
const emptyArray: number[] = [];
const singleElementArray = [3];
const array = [-5, -3, 0, 1, 1, 8, 8, 8, 100];
test("returns null if collection is empty with lowIdx < highIdx", () => {
const result = upperBoundBinarySearch((idx) => emptyArray[idx], 0, 1, 5);
expect(result).toBeNull();
});
test("returns null if collection is empty with highIdx < lowIdx", () => {
const result = upperBoundBinarySearch((idx) => emptyArray[idx], 0, -1, 5);
expect(result).toBeNull();
});
test("returns the first index if upperboundValue < all elements in array", () => {
const result = upperBoundBinarySearch((idx) => array[idx], 0, array.length - 1, -10);
expect(result).toBe(0);
});
test("returns the first index if upperboundValue < element in singleElementArray", () => {
const result = upperBoundBinarySearch((idx) => singleElementArray[idx], 0, 0, -10);
expect(result).toBe(0);
});
test("returns the last index if upperboundValue > all elements in array", () => {
const result = upperBoundBinarySearch((idx) => array[idx], 0, array.length - 1, 200);
expect(result).toBe(array.length - 1);
});
test("returns the last index if upperboundValue > element in singleElementArray", () => {
const result = upperBoundBinarySearch((idx) => singleElementArray[idx], 0, 0, 200);
expect(result).toBe(0);
});
test("returns the correct index if upperboundValue doesn't match any elements", () => {
const result = upperBoundBinarySearch((idx) => array[idx], 0, array.length - 1, 2);
expect(result).toBe(4);
});
test("returns the correct index if upperboundValue matches a unique element", () => {
const result = upperBoundBinarySearch((idx) => array[idx], 0, array.length - 1, 0);
expect(result).toBe(2);
});
test("returns the last matched index if upperboundValue matches a duplicated element", () => {
const result = upperBoundBinarySearch((idx) => array[idx], 0, array.length - 1, 8);
expect(result).toBe(7);
});
test("returns index 0 if upperboundValue matches the first element", () => {
const result = upperBoundBinarySearch((idx) => array[idx], 0, array.length - 1, -5);
expect(result).toBe(0);
});
test("returns the last index if upperboundValue matches the last element", () => {
const result = upperBoundBinarySearch((idx) => array[idx], 0, array.length - 1, 100);
expect(result).toBe(array.length - 1);
});
});
Loading