Added debug-mode through query params #380
Conversation
WalkthroughThis update enhances search functionality and debug management across both frontend and backend modules. In the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SearchComponent
participant useEffect
participant Navigation
User->>SearchComponent: Initiate search action
SearchComponent->>useEffect: Evaluate debug info (env var & search.debug)
useEffect-->>SearchComponent: Update showDebugInfo state
SearchComponent->>Navigation: Navigate with query & debug flag
sequenceDiagram
participant Client
participant API
participant searchVespa
Client->>API: Send search request with debug flag
API->>searchVespa: Forward query config including `requestDebug`
searchVespa->>searchVespa: Determine `isDebugMode` (using config.isDebugMode || requestDebug)
searchVespa-->>API: Return search results with debug details (if enabled)
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 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 (
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hello @SahilKumar000, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
This pull request introduces a debug mode that can be activated via a query parameter. This allows enabling debug information even when the global debug_mode is set to false. The changes involve modifying the search.tsx file in the frontend and search.ts and vespa.ts in the server to handle the new debug query parameter.
Highlights
- Debug Mode via Query Parameter: The primary change is the addition of a
debugquery parameter that, when set totrue, enables debug mode, overriding the globalVITE_SHOW_DEBUG_INFOenvironment variable. - Frontend Changes: The frontend code in
search.tsxis modified to read thedebugquery parameter and update theshowDebugInfostate accordingly. Thedebugflag is also passed to the backend. - Backend Changes: The backend API in
search.tsis updated to extract thedebugquery parameter and pass it to thesearchVespafunction. ThesearchVespafunction invespa.tsis modified to accept thedebugparameter and enable trace-level logging if it's set totrue. - Schema Validation: The
searchParamsschema insearch.tsxis updated to include adebugboolean parameter with a default value offalse.
Changelog
Click here to see the changelog
- frontend/src/routes/_authenticated/search.tsx
- Modified the
showDebugInfostate initialization to consider thedebugquery parameter. - Added
setDebugInfoto update debug info based on query params - Added
searchas a dependency to theuseEffecthook to update the query. - Passed the
showDebugInfoflag to the backend. - Updated the
searchParamsschema to include adebugboolean parameter.
- Modified the
- server/api/search.ts
- Extracted the
debugquery parameter from the request. - Passed the
debugparameter to thesearchVespafunction in bothSearchApiandAnswerApi.
- Extracted the
- server/search/vespa.ts
- Modified the
searchVespafunction to accept arequestDebugboolean parameter. - Enabled trace-level logging in Vespa if either the global
config.isDebugModeor therequestDebugparameter istrue.
- Modified the
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
A query's tale,
Debug's flag, now set to sail,
Tracing through the night.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces a debug mode that can be enabled via a query parameter, which is a useful feature for debugging purposes. The changes look good overall, but there are a few areas that could be improved for clarity and maintainability.
Summary of Findings
- Inconsistent debug flag usage: The debug flag is being checked in multiple places, which could lead to inconsistencies if not managed carefully. Consider centralizing the debug flag check to ensure consistent behavior throughout the application.
- Potential security risk: Enabling debug mode via a query parameter could expose sensitive information if not properly secured. Ensure that debug mode is only enabled for authorized users or environments.
Merge Readiness
The pull request is almost ready for merging, but there are a few issues that need to be addressed before merging. I would recommend addressing the potential security risk and centralizing the debug flag check to ensure consistent behavior throughout the application. I am unable to approve this pull request, and recommend that others review and approve this code before merging.
| const [showDebugInfo, setDebugInfo] = useState( | ||
| import.meta.env.VITE_SHOW_DEBUG_INFO === "true" || search.debug || false, |
There was a problem hiding this comment.
Consider using a more descriptive name for the state variable. setDebugInfo is good, but isDebugMode would be more explicit about what the variable represents. Also, it might be beneficial to initialize this state using a function to avoid re-renders when the component mounts, especially if search.debug is expensive to compute.
const [isDebugMode, setIsDebugMode] = useState(
() => import.meta.env.VITE_SHOW_DEBUG_INFO === "true" || search.debug || false,
)![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
frontend/src/routes/_authenticated/search.tsx (1)
106-108: Good addition, but consider extracting the debug logic into a helper function.The initialization of
showDebugInfonow correctly includes thesearch.debugvalue from URL parameters, which enables debug mode via query params.Consider extracting this debug logic into a helper function since it's duplicated at line 180:
+const getDebugModeEnabled = (search: any) => + import.meta.env.VITE_SHOW_DEBUG_INFO === "true" || search?.debug || false; const [showDebugInfo, setDebugInfo] = useState( - import.meta.env.VITE_SHOW_DEBUG_INFO === "true" || search.debug || false, + getDebugModeEnabled(search) )server/api/search.ts (3)
143-146: Properly extract the debug flag from query parameters.The extraction of the debug parameter from the request is implemented correctly.
Consider removing the
@ts-ignorecomment by properly typing the request parameters:let { query, groupCount: gc, offset, page, app, entity, lastUpdated, isQueryTyped, debug, // Extract debug query param - // @ts-ignore } = c.req.valid("query")
165-169: Debug parameter correctly passed to searchVespa.The debug flag is now properly passed to the searchVespa function.
Consider using named parameters to improve code readability, especially when there are many undefined parameters:
searchVespa( decodedQuery, email, app, entity, page, offset, 0.5, timestampRange, undefined, undefined, undefined, debug, ) // Could be refactored to use an options object pattern in future improvements
211-213: Debug mode passed to searchVespa in AnswerApi.The AnswerApi is correctly passing the global debug setting from config.
For consistency with SearchApi, consider extracting the debug parameter from the request query parameters:
- searchVespa(decodedQuery, email, app, entity, config.answerPage, 0, 0.5, undefined, undefined, undefined, undefined, config.isDebugMode) + // @ts-ignore + const { debug } = c.req.valid("query") + searchVespa(decodedQuery, email, app, entity, config.answerPage, 0, 0.5, undefined, undefined, undefined, undefined, debug || config.isDebugMode)server/search/vespa.ts (2)
446-447: Good addition of the requestDebug parameter.The new optional parameter correctly enables client-side control of debug mode.
Remove the unnecessary parentheses around the default value:
- requestDebug: boolean = (false), + requestDebug: boolean = false,
450-451: Properly combines global and request debug flags.The new
isDebugModevariable correctly combines both the global configuration and the request-specific debug flag.The
|| falseat the end is redundant since the expression will already return a boolean:- const isDebugMode = config.isDebugMode || requestDebug || false; + const isDebugMode = config.isDebugMode || requestDebug;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
frontend/src/routes/_authenticated/search.tsx(4 hunks)server/api/search.ts(4 hunks)server/search/vespa.ts(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
server/api/search.ts (1)
server/search/vespa.ts (1)
searchVespa(434-490)
server/search/vespa.ts (1)
server/search/types.ts (1)
VespaSearchResponse(670-670)
🔇 Additional comments (5)
frontend/src/routes/_authenticated/search.tsx (3)
180-182: useEffect dependency handling looks good.The effect correctly updates the debug state when the search parameters change, enabling dynamic toggling of debug mode.
248-249: Good implementation passing debug state to the search params.Passing
showDebugInfoto the API call ensures the debug mode setting is properly communicated to the backend.
535-536: Good schema update for the new debug parameter.The addition of the debug property to the schema with appropriate defaults ensures type safety and validation for the new feature.
server/api/search.ts (1)
187-191: Debug parameter correctly passed in the else block.The debug flag is consistently passed to searchVespa in the else block.
server/search/vespa.ts (1)
477-480: Correctly uses isDebugMode for conditional debugging properties.The payload now properly includes debug properties based on the combined debug mode state.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
server/tests/search.test.ts (1)
10-236: Consider adding tests for the new debug functionality.While the test updates correctly reflect the changed
searchVespasignature, none of the tests validate the newrequestDebugfunctionality that was added to the API. Consider adding a test case that verifies the behavior whenrequestDebugis set totrue.// Add a new test case to test debug mode functionality + test("search with debug mode enabled", async () => { + const query = "test query with debug"; + // Use type assertion to access the debug-specific fields + const searchResults = await searchVespa(query, user, null, null, { + limit: 10, + requestDebug: true + }); + + // Verify that debug information is included in the response + // For example, check for the presence of ranking features or trace information + expect(searchResults.root.hasOwnProperty('featureNames')).toBeTrue(); + // or other assertions specific to what debug mode should return + });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
frontend/src/routes/_authenticated/search.tsx(5 hunks)server/api/chat.ts(7 hunks)server/api/search.ts(4 hunks)server/search/vespa.ts(2 hunks)server/tests/search.test.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- server/api/chat.ts
- server/api/search.ts
- frontend/src/routes/_authenticated/search.tsx
- server/search/vespa.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
server/tests/search.test.ts (1)
server/search/vespa.ts (1)
searchVespa(445-501)
🔇 Additional comments (3)
server/tests/search.test.ts (3)
26-26: Good update to the searchVespa function call.The function call has been correctly updated to use the new object-based parameter structure with the
limitproperty. This aligns with the recent changes to thesearchVespafunction signature inserver/search/vespa.tswhere a single object parameter is now accepted instead of multiple positional parameters.
196-197: Function call correctly updated to use object parameter.The call to
searchVespahas been properly updated to use the new parameter structure. This is consistent with the function's updated signature inserver/search/vespa.ts.
214-215: Function call correctly updated to use object parameter.The searchVespa function call has been properly updated to use the new object parameter with the
limitproperty, consistent with other changes in this file.
Description
Added debug-mode through query params Now if Global debug_mode is false then we can explicitly add a query param as &debug=true to enable debug mode
Testing
Additional Notes
Summary by CodeRabbit
New Features
Refactor
Tests