feat(new-webui): Limit search results to most recent 1000 entries.#999
Conversation
WalkthroughThe changes update the search results sorting logic to order by timestamp descending and, in case of ties, by ID descending. A new constant for the maximum number of search results is introduced and used to limit query results. The relevant hook is updated to apply these sorting and limiting options. Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)`**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}`: - Prefer `false == ` rather than `!`.
🧬 Code Graph Analysis (1)components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/useSearchResults.ts (2)
⏰ Context from checks skipped due to timeout of 90000ms (2)
🔇 Additional comments (1)
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
There was a problem hiding this comment.
Actionable comments posted: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/typings.tsx(2 hunks)components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/useSearchResults.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}`: - Prefer `false == ` rather than `!`.
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}: - Preferfalse == <expression>rather than!<expression>.
components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/useSearchResults.tscomponents/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/typings.tsx
🧬 Code Graph Analysis (2)
components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/useSearchResults.ts (1)
components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/typings.tsx (1)
SEARCH_MAX_NUM_RESULTS(86-86)
components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/typings.tsx (2)
components/log-viewer-webui/client/src/typings/datetime.ts (1)
DATETIME_FORMAT_TEMPLATE(3-3)components/log-viewer-webui/server/src/fastify-v2/routes/api/search/typings.ts (1)
SEARCH_MAX_NUM_RESULTS(47-47)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: lint-check (ubuntu-latest)
- GitHub Check: build (macos-latest)
- GitHub Check: lint-check (macos-latest)
| SEARCH_MAX_NUM_RESULTS, | ||
| searchResultsTableColumns, | ||
| TABLE_BOTTOM_PADDING, | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Re-export order nit
Minor: exporting the new constant before the column configs would better match the declaration order above.
🤖 Prompt for AI Agents
In
components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/typings.tsx
around lines 86 to 89, the export order is inconsistent with the declaration
order. Move the export of SEARCH_MAX_NUM_RESULTS to appear before the export of
searchResultsTableColumns and TABLE_BOTTOM_PADDING to match the order in which
they are declared above.
| /** | ||
| * The maximum number of results to retrieve for a search. | ||
| */ | ||
| const SEARCH_MAX_NUM_RESULTS = 1000; | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Duplicate definition of SEARCH_MAX_NUM_RESULTS across client & server
A constant with the same value already lives in server/src/fastify-v2/routes/api/search/typings.ts. Maintaining two sources invites drift.
Recommend moving the value to a shared package (e.g., shared/consts.ts) and importing it in both places, or piping it from the server to the client via an API/ENV.
🤖 Prompt for AI Agents
In
components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/typings.tsx
around lines 78 to 82, the constant SEARCH_MAX_NUM_RESULTS is duplicated from
the server code, risking value drift. To fix this, move the constant definition
to a shared module like shared/consts.ts, then import it in both the client and
server files. Alternatively, expose the value from the server via an API or
environment variable and consume it in the client to maintain a single source of
truth.
| const timestampDiff = a.timestamp - b.timestamp; | ||
| if (0 !== timestampDiff) { | ||
| return timestampDiff; | ||
| } | ||
|
|
||
| return a._id.localeCompare(b._id); | ||
| }, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Comparator returns ascending order; verify this aligns with intended “descend” default
defaultSortOrder is set to "descend", yet the comparator returns a.timestamp - b.timestamp, which produces an ascending result set that Ant Design will then invert to achieve descending order.
If someone later removes/changes defaultSortOrder, the implicit coupling may break. Consider returning the inverted difference to make the comparator self-contained and future-proof:
-const timestampDiff = a.timestamp - b.timestamp;
+const timestampDiff = b.timestamp - a.timestamp;Same reasoning applies to the _id fallback:
-return a._id.localeCompare(b._id);
+return b._id.localeCompare(a._id);This keeps the comparator’s semantics clear and avoids relying on external props for direction.
🏁 Script executed:
#!/bin/bash
# Search for defaultSortOrder occurrences across the codebase
rg -n "defaultSortOrder" -A3 -B3
echo "----"
# Search for sorter definitions in SearchResultsTable directory
rg -n "sorter" -A5 -B5 components/log-viewer-webui/client/src/pages/SearchPage/SearchResultsLength of output: 2691
Make the sorter self-contained by flipping to descending
The current comparator (a.timestamp - b.timestamp) produces an ascending sort that Ant Design then reverses due to defaultSortOrder: "descend". To avoid this implicit coupling—so changing or removing defaultSortOrder won’t silently break the order—flip both parts of the comparator to sort descending by default:
• File:
components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/typings.tsx
• Around lines 36–43
- sorter: (a, b) => {
- const timestampDiff = a.timestamp - b.timestamp;
+ sorter: (a, b) => {
+ const timestampDiff = b.timestamp - a.timestamp;
if (0 !== timestampDiff) {
return timestampDiff;
}
-
- return a._id.localeCompare(b._id);
+ return b._id.localeCompare(a._id);
},This change makes the comparator’s intent explicit and decoupled from the defaultSortOrder prop.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const timestampDiff = a.timestamp - b.timestamp; | |
| if (0 !== timestampDiff) { | |
| return timestampDiff; | |
| } | |
| return a._id.localeCompare(b._id); | |
| }, | |
| sorter: (a, b) => { | |
| const timestampDiff = b.timestamp - a.timestamp; | |
| if (0 !== timestampDiff) { | |
| return timestampDiff; | |
| } | |
| return b._id.localeCompare(a._id); | |
| }, |
🤖 Prompt for AI Agents
In
components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/typings.tsx
around lines 36 to 43, the comparator currently returns ascending order by
subtracting timestamps (a.timestamp - b.timestamp) and using localeCompare on
_id, which relies on the external defaultSortOrder "descend" to invert the
order. To make the comparator self-contained and explicitly descending, invert
the subtraction to (b.timestamp - a.timestamp) and reverse the localeCompare to
b._id.localeCompare(a._id). This ensures the comparator inherently sorts in
descending order without depending on external props.
| // Retrieve 1k most recent results. | ||
| const options = { | ||
| sort: [ | ||
| [ | ||
| "timestamp", | ||
| "desc", | ||
| ], | ||
| [ | ||
| "id", | ||
| "desc", | ||
| ], | ||
| ], | ||
| limit: SEARCH_MAX_NUM_RESULTS, | ||
| }; | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Possible micro-optimisation: build options outside cursor factory
options is constant; creating the object on every invocation of the factory is unnecessary. Hoist it above the callback to avoid needless allocations.
const BASE_SEARCH_OPTIONS = {
sort: [
["timestamp", "desc"],
["_id", "desc"],
],
limit: SEARCH_MAX_NUM_RESULTS,
};
...
const searchResultsCursor = useCursor<SearchResult>(
() => {
if (searchJobId === SEARCH_STATE_DEFAULT.searchJobId) {
return null;
}
const collection = new MongoCollectionSocket(searchJobId.toString());
return collection.find({}, BASE_SEARCH_OPTIONS);
},
[searchJobId]
);🤖 Prompt for AI Agents
In
components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/useSearchResults.ts
around lines 26 to 40, the options object used for sorting and limiting search
results is recreated on every invocation of the cursor factory, causing
unnecessary allocations. To fix this, move the options object outside the
factory function and define it as a constant at the module level. Then,
reference this constant inside the factory to avoid rebuilding the object
repeatedly.
junhaoliao
left a comment
There was a problem hiding this comment.
For the PR title, how about:
feat(new-webui): Limit search results to most recent 1000 entries.
Description
Per discussion, results cache can actually have >1000 entries.
PR limits search results to 1000 most recent entries. Data is still sorted client side, but we only sort 1000 recent entries.
I also added fallback to sort by mongoID when there are ties.
Checklist
breaking change.
Validation performed
Only retrieves 1000 results.
Summary by CodeRabbit
New Features
Enhancements