new-log-viewer/src/utils/time.ts (1)

1-4: Improve function documentation

The current documentation is incomplete. Consider enhancing it as follows:

1. Add a description of the function's purpose and behaviour.
2. Complete the @param tag with details about the callback parameter.
3. Include a @returns tag to explicitly state that the function doesn't return a value.

Here's a suggested improvement:

/**
 * Defers the execution of a callback function to the next event loop cycle.
 * 
 * @param callback - The function to be executed in the next event loop cycle.
 * @returns {void}
 */

new-log-viewer/src/components/MenuBar/index.tsx (1)

35-35: LGTM: Search feature added to MenuBar

The changes to add the search feature to the MenuBar component look good:

1. The queryLogs function is correctly added to the destructured context.
2. The new search button is logically placed at the end of the menu bar.
3. The onClick handler for the new button correctly uses the queryLogs function.
4. The button uses the SmallIconButton component, maintaining consistency with other buttons in the menu bar.

Consider adding an aria-label to the new button to improve accessibility. For example:

 <SmallIconButton
     onClick={queryLogs}
+    aria-label="Search logs"
 >
     <SearchIcon/>
 </SmallIconButton>

Also applies to: 82-86

new-log-viewer/src/typings/worker.ts (1)

76-82: Consider exporting ChunkResultType for external use

While ChunkResults is exported, ChunkResultType might also need to be exported if it's used elsewhere in the application. This ensures that all necessary types are accessible where required.

You can modify the export statement as follows:

 export type {
     BeginLineNumToLogEventNumMap,
     ChunkResults,
+    ChunkResultType,
     CursorType,
     FileSrcType,
     MainWorkerReqMessage,
     MainWorkerRespMessage,
     WorkerReq,
     WorkerResp,
 };

new-log-viewer/src/services/MainWorker.ts (1)

46-48: Ensure non-blocking query processing and result aggregation

To improve responsiveness, consider processing query tasks non-blockingly by submitting them one-by-one using setTimeout() or a similar mechanism. This allows the renderer to handle incoming requests between task executions. Also, ensure that the rendering component aggregates all results upon receiving each chunk to keep the user interface updated.

Would you like assistance in refactoring the query implementation to align with these design recommendations?

Also applies to: 107-116

Configuration used: CodeRabbit UI
Review profile: CHILL

new-log-viewer/src/services/LogFileManager.ts (1)

Line range hint 58-88: LGTM: New fields and constructor updates

The new private fields and constructor updates are well-aligned with the new query functionality. However, there's a minor optimization opportunity:

Consider removing the initial value of #numEvents:

- readonly #numEvents: number = 0;
+ readonly #numEvents: number;

This field is set in the constructor, so the initial value of 0 is unnecessary.

Configuration used: CodeRabbit UI
Review profile: CHILL

new-log-viewer/src/components/MenuBar/index.tsx (1)

86-90: Enhance user feedback and accessibility for the search button

The search button is correctly implemented, but could benefit from some enhancements:

1. Add a tooltip to explain the button's function.
2. Include an aria-label for better accessibility.
3. Consider adding visual feedback when a search is in progress.

Here's a suggested improvement:

<SmallIconButton
  onClick={handleSearchButtonClick}
  title="Search Logs"
  aria-label="Search Logs"
  disabled={isLoading}
>
  {isLoading ? <CircularProgress size={24} /> : <SearchIcon />}
</SmallIconButton>

This change adds a tooltip, improves accessibility, and provides visual feedback during the search process.

new-log-viewer/src/contexts/StateContextProvider.tsx (1)

218-222: Simplify object property assignment using shorthand notation

In the queryLogs function, you can use the object property shorthand to make the code more concise since the property names and variables are the same.

Apply this diff to simplify the code:

workerPostReq(mainWorkerRef.current, WORKER_REQ_CODE.QUERY_LOG, {
-    searchString: searchString,
-    isRegex: isRegex,
-    isCaseSensitive: isCaseSensitive,
+    searchString,
+    isRegex,
+    isCaseSensitive,
});

Configuration used: CodeRabbit UI
Review profile: CHILL

new-log-viewer/src/utils/time.ts (1)

6-8: The implementation looks good. Consider adding a type check for robustness.

The defer function correctly uses setTimeout to defer the execution of the callback function. The implementation is concise and effective.

For added robustness, you might consider adding a type check to ensure callbackFn is a function:

 const defer = (callbackFn: () => void) => {
+    if (typeof callbackFn !== 'function') {
+        throw new TypeError('callbackFn must be a function');
+    }
     setTimeout(callbackFn, 0);
 };

This change would provide clearer error messages if defer is called with an invalid argument.

new-log-viewer/src/services/LogFileManager/index.ts (5)

15-24: LGTM! Good use of imports and constants.

The new imports and constants are well-organized and relevant to the added functionality. Using SEARCH_CHUNK_SIZE from a config file is a good practice for maintainability.

Consider adding a brief comment explaining the purpose of SEARCH_CHUNK_SIZE in the config file for better documentation.

---

Line range hint 46-125: LGTM! Constructor and create method updated correctly.

The changes to the constructor and create method are well-implemented and consistent with the new query functionality. The use of private fields and the callback pattern for query results is appropriate.

However, there's a minor documentation issue:

Please update the JSDoc for the constructor to include the @param for onQueryResults:

/**
 * Private constructor for LogFileManager. This is not intended to be invoked publicly.
 * Instead, use LogFileManager.create() to create a new instance of the class.
 *
 * @param params
 * @param params.decoder
 * @param params.fileName
 * @param params.onDiskFileSizeInBytes
 * @param params.pageSize Page size for setting up pagination.
 * @param params.onQueryResults Callback function to handle query results.
 */

🧰 Tools

🪛 GitHub Check: lint-check

[warning] 54-54:
Missing JSDoc @param "params.onQueryResults" declaration

---

[warning] 58-58:
Missing @param "params.onQueryResults"

---

279-305: LGTM! Well-implemented startQuery method.

The startQuery method is well-structured and handles different cases appropriately. The use of #queryId to manage ongoing searches is a good practice.

For improved readability, consider extracting the regex construction into a separate private method:

private constructSearchRegex(searchString: string, isRegex: boolean, isCaseSensitive: boolean): RegExp {
    const regexPattern = isRegex ?
        searchString :
        searchString.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
    const regexFlags = isCaseSensitive ? "" : "i";
    return new RegExp(regexPattern, regexFlags);
}

Then use it in startQuery:

const searchRegex = this.constructSearchRegex(searchString, isRegex, isCaseSensitive);

This change would make the startQuery method more concise and easier to understand.

---

307-355: LGTM! Well-implemented chunk-based search, but consider using setTimeout.

The #searchChunkAndScheduleNext method effectively implements chunk-based searching, which is great for handling large log files without blocking the main thread. The use of queryId to manage ongoing searches is a good practice.

However, consider replacing defer with setTimeout for scheduling the next chunk search. setTimeout is more widely supported and its behaviour is more predictable across different JavaScript environments:

if (endSearchIdx < this.#numEvents) {
    setTimeout(() => {
        this.#searchChunkAndScheduleNext(queryId, endSearchIdx, searchRegex);
    }, 0);
}

This change will maintain the same non-blocking behaviour while using a more standard JavaScript function.

---

54-54: Add missing JSDoc @param for onQueryResults.

To complete the documentation, please add the missing JSDoc @param for params.onQueryResults in the constructor:

/**
 * Private constructor for LogFileManager. This is not intended to be invoked publicly.
 * Instead, use LogFileManager.create() to create a new instance of the class.
 *
 * @param params
 * @param params.decoder
 * @param params.fileName
 * @param params.onDiskFileSizeInBytes
 * @param params.pageSize Page size for setting up pagination.
 * @param params.onQueryResults Callback function to handle query results.
 */

This will ensure that all parameters are properly documented, improving code maintainability.

🧰 Tools

🪛 GitHub Check: lint-check

[warning] 54-54:
Missing JSDoc @param "params.onQueryResults" declaration

Configuration used: CodeRabbit UI
Review profile: CHILL

new-log-viewer/src/services/LogFileManager/index.ts (3)

40-50: LGTM! Good additions for query functionality.

The changes to the class properties are well-aligned with the new query functionality. Making #fileName readonly is a good practice for immutability. The new properties #queryId and #onQueryResults are necessary for managing queries.

Consider using a more descriptive name for #queryId, such as #currentQueryId or #latestQueryId, to better convey its purpose.

---

280-306: LGTM! Well-implemented query initiation logic.

The startQuery method effectively implements the logic for starting a new query. It properly handles empty query strings and the case of no logs. The RegExp construction looks correct and accounts for both regex and non-regex queries.

Consider moving the this.#queryId++ increment to the beginning of the method to ensure it's always incremented, even for empty queries. This would prevent potential issues with query ID management.

---

308-356: LGTM! Well-implemented chunk querying logic, with room for improvements.

The #queryChunkAndScheduleNext method effectively implements the core logic for querying log chunks. It properly handles query cancellation and chunk size using the QUERY_CHUNK_SIZE constant.

Consider the following improvements:

1. Add explicit error handling for the decodeRange call to handle potential decoding failures.
2. Replace defer with setTimeout for better cross-browser compatibility.
3. Simplify the nested ternary operator in the matchResult check for better readability.

Example for point 3:

if (null !== matchResult && "number" === typeof matchResult.index) {
    // Rest of the code
}

new-log-viewer/src/contexts/StateContextProvider.tsx (1)

303-318: LGTM! Consider clearing existing query results before starting a new query

The implementation of startQuery looks good and correctly handles the case where mainWorkerRef.current is null. However, it might be beneficial to clear any existing query results before starting a new query to ensure a clean slate for each search.

Consider adding the following line at the beginning of the function:

setQueryResults(new Map());

This will reset the query results before initiating a new search, preventing any potential confusion with results from previous queries.

Configuration used: CodeRabbit UI
Review profile: CHILL