V4 #311
V4 #311
Conversation
WalkthroughThis update introduces a comprehensive set of changes centered on enhanced authentication, authorization, and public access features in Sourcebot. Major additions include API key management, public access controls, support for guest users, expanded entitlements, and improved membership workflows. The update also brings new database schema migrations, documentation updates, UI enhancements, and stricter role-based access control. Numerous files were refactored to support user ID-based authentication, API key validation, and entitlement-based feature gating. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WebApp
participant API
participant DB
participant Entitlement
participant Email
participant GuestUser
User->>WebApp: Request access / login
WebApp->>Entitlement: Check public access entitlement
alt Public access enabled
WebApp->>GuestUser: Create/ensure guest user
WebApp->>API: Authenticate as guest
API->>DB: Grant GUEST role
API-->>WebApp: Access granted
WebApp-->>User: Show guest UI
else Not public access
WebApp->>API: Authenticate with user credentials or API key
API->>DB: Validate user/org membership
alt User is first member
API->>DB: Assign OWNER role, clear pendingApproval
else User is not first member
API->>Entitlement: Check JIT provisioning
alt JIT enabled and seats available
API->>DB: Provision user as MEMBER
else JIT not enabled or seats full
API->>DB: Create AccountRequest (pendingApproval)
API->>Email: Notify org owner(s)
WebApp-->>User: Show pending approval UI
end
end
API-->>WebApp: Access granted or pending
WebApp-->>User: Show appropriate UI
end
sequenceDiagram
participant User
participant WebApp
participant API
participant DB
User->>WebApp: Request API key management
WebApp->>API: Create/list/delete API key (with userId)
API->>DB: Validate user/org, manage ApiKey table
API-->>WebApp: Return API key data or errors
WebApp-->>User: Show API key UI and feedback
Possibly related PRs
Suggested reviewers
Poem
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
docs/self-hosting/upgrade/v3-to-v4-guide.mdx (5)
2-3: Consistency: Capitalize "Guide" in sidebarTitleThe
sidebarTitleis currently set to "V3 to V4 guide" (lowercase "guide"), whiletitleuses "V3 to V4 Guide". For UI consistency, consider:-sidebarTitle: V3 to V4 guide +sidebarTitle: V3 to V4 Guide
6-6: Consistency: Capitalize version identifiersThe intro uses lowercase "v3" and "v4", but the title uses uppercase "V3" and "V4". For consistency, change to:
-This guide will walk you through upgrading your Sourcebot deployment from v3 to v4. +This guide will walk you through upgrading your Sourcebot deployment from V3 to V4.
10-11: Grammar: Pluralize "seat"The text reads "unlimited seat [enterprise license]" but should be plural:
-- Unauthorized access to a Sourcebot deployment. Authentication is now required, and unauthorized access requires an unlimited seat [enterprise license] +- Unauthorized access to a Sourcebot deployment. Authentication is now required, and unauthorized access requires unlimited seats [enterprise license]
27-28: Consistency: Align image rendering syntaxThe guide mixes raw
<img>tags (lines 27–28, 32–33) with Markdown image syntax (lines 37–38). For consistency, standardize on one approach—for example, convert the Markdown image to an<img>tag:- +<img alt="Pending Approval Page" width="600" style={{ borderRadius: '0.5rem' }} src="/images/pending_approval.png" />Also applies to: 32-33, 37-38
49-50: Grammar: Remove redundant "by"The current phrasing "approved by the owner by navigating" is awkward. Consider simplifying:
-The only change that's required if your deployment has authentication enabled is to unset the `SOURCEBOT_AUTH_ENABLED` environment variable. New user registrations will now submit a request to join the organization which can be approved by the owner by navigating to **Settings -> Members**. +The only change required if your deployment has authentication enabled is to unset the `SOURCEBOT_AUTH_ENABLED` environment variable. New user registrations will now submit a request to join the organization, which can be approved by the owner via **Settings -> Members**.🧰 Tools
🪛 LanguageTool
[uncategorized] ~49-~49: Possible missing comma found.
Context: ...s will now submit a request to join the organization which can be approved by the owner by ...(AI_HYDRA_LEO_MISSING_COMMA)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/self-hosting/upgrade/v3-to-v4-guide.mdx(1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/self-hosting/upgrade/v3-to-v4-guide.mdx
[uncategorized] ~49-~49: Possible missing comma found.
Context: ...s will now submit a request to join the organization which can be approved by the owner by ...
(AI_HYDRA_LEO_MISSING_COMMA)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (1)
docs/self-hosting/upgrade/v3-to-v4-guide.mdx (1)
54-55: Verified: Internal link fragment is correctThe link to the single-tenant section uses
#if-your-deployment-doesnt-have-authentication-enabled, which matches the header ID above. No changes needed here.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
packages/web/src/actions.ts (2)
447-478: Race condition still exists in API key creation.The check-then-create pattern (lines 450-463 then 466-473) creates a race condition where two concurrent requests could both pass the existence check and then both try to create the same API key.
As suggested in previous reviews, this should rely on database constraints and handle unique constraint violations:
export const createApiKey = async (name: string, domain: string): Promise<{ key: string } | ServiceError> => sew(() => withAuth((userId) => withOrgMembership(userId, domain, async ({ org }) => { - const existingApiKey = await prisma.apiKey.findFirst({ - where: { - createdById: userId, - name, - }, - }); - - if (existingApiKey) { - return { - statusCode: StatusCodes.BAD_REQUEST, - errorCode: ErrorCode.API_KEY_ALREADY_EXISTS, - message: `API key ${name} already exists`, - } satisfies ServiceError; - } - const { key, hash } = generateApiKey(); - await prisma.apiKey.create({ - data: { - name, - hash, - orgId: org.id, - createdById: userId, - } - }); - - return { - key, - } + + try { + await prisma.apiKey.create({ + data: { + name, + hash, + orgId: org.id, + createdById: userId, + } + }); + return { key }; + } catch (e) { + if (e instanceof Prisma.PrismaClientKnownRequestError && e.code === 'P2002') { + return { + statusCode: StatusCodes.BAD_REQUEST, + errorCode: ErrorCode.API_KEY_ALREADY_EXISTS, + message: `API key ${name} already exists`, + } satisfies ServiceError; + } + throw e; + } })));
1301-1312: Pendingapproval semantics issue still unresolved.The TODO comments acknowledge the exact issue raised in previous reviews - setting
pendingApproval = trueunconditionally creates problems in multi-tenant mode where orphaned users can never be approved.As suggested previously, consider making this conditional based on tenancy mode:
- await tx.user.update({ - where: { - id: memberId, - }, - data: { - pendingApproval: true, - } - }); + await tx.user.update({ + where: { + id: memberId, + }, + data: { + pendingApproval: env.SOURCEBOT_TENANCY_MODE === 'single', + } + });Or introduce a dedicated "removedByOwner" flag to distinguish removed members.
🧹 Nitpick comments (2)
packages/web/src/actions.ts (2)
57-110: Consider refactoring for improved readability.The authentication logic is correct but quite complex with multiple authentication paths. The API key authentication and public access features are properly implemented with appropriate security checks.
Consider extracting authentication methods into separate functions to improve readability:
export const withAuth = async <T>(fn: (userId: string) => Promise<T>, allowSingleTenantUnauthedAccess: boolean = false, apiKey: ApiKeyPayload | undefined = undefined) => { const session = await auth(); if (!session) { - // First we check if public access is enabled and supported. If not, then we check if an api key was provided. If not, - // then this is an invalid unauthed request and we return a 401. - const publicAccessEnabled = await getPublicAccessStatus(SINGLE_TENANT_ORG_DOMAIN); - if (apiKey) { - const apiKeyOrError = await verifyApiKey(apiKey); - // ... rest of API key logic - } else if ( - env.SOURCEBOT_TENANCY_MODE === 'single' && - allowSingleTenantUnauthedAccess && - !isServiceError(publicAccessEnabled) && - publicAccessEnabled - ) { - // ... rest of public access logic - } + return await handleUnauthenticatedRequest(apiKey, allowSingleTenantUnauthedAccess, fn); } return fn(session.user.id); }
1448-1556: Well-implemented account request system with good email integration.The account request creation function properly handles validation, prevents duplicates, and sends appropriate email notifications to organization owners.
Consider wrapping with
withAuthfor consistency with other functions, or document why directuserIdparameter is used instead.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/web/src/actions.ts(53 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/web/src/actions.ts (11)
packages/web/src/lib/types.ts (1)
ApiKeyPayload(14-17)packages/web/src/ee/features/publicAccess/publicAccess.tsx (1)
getPublicAccessStatus(14-39)packages/web/src/lib/constants.ts (4)
SINGLE_TENANT_ORG_DOMAIN(32-32)SOURCEBOT_SUPPORT_EMAIL(35-35)SOURCEBOT_GUEST_USER_ID(29-29)MOBILE_UNSUPPORTED_SPLASH_SCREEN_DISMISSED_COOKIE_NAME(25-25)packages/web/src/lib/utils.ts (1)
isServiceError(190-196)packages/web/src/lib/serviceError.ts (4)
notAuthenticated(83-89)notFound(91-97)ServiceError(11-11)ServiceErrorException(16-20)packages/web/src/features/entitlements/server.ts (4)
hasEntitlement(68-71)getPlan(40-61)getSeats(63-66)SOURCEBOT_UNLIMITED_SEATS(8-8)packages/web/src/ee/features/billing/stripe.ts (1)
IS_BILLING_ENABLED(6-6)packages/web/src/ee/features/billing/serverUtils.ts (2)
getSubscriptionForOrg(53-80)decrementOrgSeatCount(31-51)packages/crypto/src/index.ts (2)
hashSecret(27-33)generateApiKey(35-47)packages/web/src/emails/joinRequestSubmittedEmail.tsx (1)
JoinRequestSubmittedEmail(32-117)packages/web/src/emails/joinRequestApprovedEmail.tsx (1)
JoinRequestApprovedEmail(29-83)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (8)
packages/web/src/actions.ts (8)
1-36: LGTM! Well-organized imports for new authentication features.The new imports properly support the API key authentication, public access, and account request management functionality.
112-139: LGTM! Proper seat availability checking.The function correctly excludes guest users from seat counts and properly enforces entitlement-based limits.
141-189: Excellent refactoring of authorization logic.The updated function properly enforces role-based access control with clear precedence rules and returns the full org object to reduce database calls.
394-444: LGTM! Secure API key verification implementation.The function properly validates the full API key by hashing the secret portion and includes appropriate domain validation. This addresses the security concerns from previous reviews.
480-527: LGTM! Well-implemented API key management functions.Both
deleteApiKeyandgetUserApiKeysproperly enforce ownership checks and have appropriate error handling.
902-914: LGTM! Important seat capacity checking added.The addition of seat availability checking prevents creating invites when the organization is at capacity, which is essential for proper license enforcement.
1558-1669: LGTM! Robust account request approval workflow.Both approval and rejection functions properly use transactions, check seat availability, and include comprehensive email notifications with appropriate error handling.
635-636: LGTM! Consistent support for guest access where appropriate.The addition of guest role support and
allowSingleTenantUnauthedAccessfor read-only operations like getting repositories and user roles is well-implemented.Also applies to: 697-698, 896-896
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
packages/web/src/actions.ts (2)
447-478: API key creation still has race condition and inconsistent error handling.This is the same issue identified in previous reviews. The function still checks for existing keys before creation, which creates a race condition between steps 1 and 2.
1301-1312: ClarifypendingApprovalsemantics when removing membersThis is the same issue identified in previous reviews. Setting
pendingApproval = trueunconditionally when removing a member creates problems in multi-tenant mode by blocking approval workflows and creating orphaned users.
🧹 Nitpick comments (2)
packages/web/src/actions.ts (2)
57-110: Consider refactoring complex authentication logic for better maintainability.The
withAuthfunction has become quite complex with multiple authentication paths (session, API key, public access). This complexity makes it harder to reason about and maintain.Consider extracting the different authentication strategies into separate functions:
+const authenticateWithSession = async (session: any) => { + return session?.user?.id ? { userId: session.user.id } : null; +}; + +const authenticateWithApiKey = async (apiKey: ApiKeyPayload) => { + const apiKeyOrError = await verifyApiKey(apiKey); + if (isServiceError(apiKeyOrError)) { + console.error(`Invalid API key: ${JSON.stringify(apiKey)}. Error: ${JSON.stringify(apiKeyOrError)}`); + return null; + } + + const user = await prisma.user.findUnique({ + where: { id: apiKeyOrError.apiKey.createdById }, + }); + + if (!user) { + console.error(`No user found for API key: ${apiKey}`); + return null; + } + + await prisma.apiKey.update({ + where: { hash: apiKeyOrError.apiKey.hash }, + data: { lastUsedAt: new Date() }, + }); + + return { userId: user.id }; +}; + +const authenticateWithPublicAccess = async (allowSingleTenantUnauthedAccess: boolean) => { + if (env.SOURCEBOT_TENANCY_MODE !== 'single' || !allowSingleTenantUnauthedAccess) { + return null; + } + + const publicAccessEnabled = await getPublicAccessStatus(SINGLE_TENANT_ORG_DOMAIN); + if (isServiceError(publicAccessEnabled) || !publicAccessEnabled) { + return null; + } + + if (!hasEntitlement("public-access")) { + const plan = getPlan(); + console.error(`Public access isn't supported in your current plan: ${plan}. If you have a valid enterprise license key, pass it via SOURCEBOT_EE_LICENSE_KEY. For support, contact ${SOURCEBOT_SUPPORT_EMAIL}.`); + return null; + } + + return { userId: SOURCEBOT_GUEST_USER_ID }; +}; export const withAuth = async <T>(fn: (userId: string) => Promise<T>, allowSingleTenantUnauthedAccess: boolean = false, apiKey: ApiKeyPayload | undefined = undefined) => { const session = await auth(); + + // Try session authentication first + if (session) { + return fn(session.user.id); + } + + // Try API key authentication + if (apiKey) { + const result = await authenticateWithApiKey(apiKey); + if (result) { + return fn(result.userId); + } + return notAuthenticated(); + } + + // Try public access authentication + const publicResult = await authenticateWithPublicAccess(allowSingleTenantUnauthedAccess); + if (publicResult) { + return fn(publicResult.userId); + } + + return notAuthenticated(); };
1614-1621: Consider using proper logging instead of console.log.The invite deletion logic uses
console.logfor what appears to be important operational information. Consider using a proper logging framework or at leastconsole.infofor better log level categorization.- console.log(`Account request approved. Deleting invite ${invite.id} for ${request.requestedBy.email}`); + console.info(`Account request approved. Deleting invite ${invite.id} for ${request.requestedBy.email}`);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/web/src/actions.ts(53 hunks)
🔇 Additional comments (2)
packages/web/src/actions.ts (2)
394-444: LGTM! API key verification properly implemented.The API key verification correctly addresses the security concerns from previous reviews. The implementation:
- ✅ Validates the key format (
sourcebot-<secret>)- ✅ Hashes the secret part using
hashSecret(parts[1])- ✅ Looks up the API key by the hash (not just prefix)
- ✅ Validates the domain matches the API key's organization
- ✅ Provides appropriate error messages
This is a secure implementation that prevents the prefix-only impersonation vulnerability mentioned in earlier reviews.
165-174: LGTM! Role precedence logic is correctly implemented.The role precedence logic correctly assigns authorization levels where higher numbers indicate higher privileges:
- GUEST (0) → lowest privileges
- MEMBER (1) → medium privileges
- OWNER (2) → highest privileges
This works well with the authorization check
getAuthorizationPrecendence(membership.role) < getAuthorizationPrecendence(minRequiredRole)to ensure users have sufficient permissions.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/web/src/ee/sso/sso.tsx (1)
145-162: Add error handling to database transaction.The transaction lacks error handling, which could lead to unhandled exceptions.
Wrap the transaction in a try/catch block to properly handle database errors:
- await prisma.$transaction(async (tx) => { - await tx.user.update({ - where: { - id: userId, - }, - data: { - pendingApproval: false, - }, - }); - - await tx.userToOrg.create({ - data: { - userId, - orgId: org.id, - role: OrgRole.MEMBER, - }, - }); - }); - - return true; + try { + await prisma.$transaction(async (tx) => { + await tx.user.update({ + where: { + id: userId, + }, + data: { + pendingApproval: false, + }, + }); + + await tx.userToOrg.create({ + data: { + userId, + orgId: org.id, + role: OrgRole.MEMBER, + }, + }); + }); + + return true; + } catch (error) { + console.error("Error during JIT provisioning:", error); + return { + statusCode: StatusCodes.INTERNAL_SERVER_ERROR, + errorCode: ErrorCode.INTERNAL_SERVER_ERROR, + message: "Failed to provision user due to a database error", + } satisfies ServiceError; + }
🧹 Nitpick comments (4)
docs/self-hosting/configuration/authentication.mdx (3)
22-22: Fix grammar: "setup" should be "set up".The word "setup" is a noun. The verb form should be spelled with a space.
-You can setup emails to be sent when new join requests are created/approved by configurating [transactional emails](/self-hosting/configuration/transactional-emails) +You can set up emails to be sent when new join requests are created/approved by configuring [transactional emails](/self-hosting/configuration/transactional-emails)🧰 Tools
🪛 LanguageTool
[grammar] ~22-~22: The word “setup” is a noun. The verb is spelled with a space.
Context: ... and approved on registration. You can setup emails to be sent when new join request...(NOUN_VERB_CONFUSION)
35-35: Improve formatting and sentence structure.When '6-digit' is used as a modifier, it should be hyphenated. Also, consider varying the sentence beginnings to improve readability.
-Email codes are 6 digit codes sent to a provided email. Email codes are enabled when transactional emails are configured using the following environment variables: +Email codes are 6-digit codes sent to a provided email address. They are enabled when transactional emails are configured using the following environment variables:🧰 Tools
🪛 LanguageTool
[grammar] ~35-~35: When ‘6-digit’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...`. ### Email codes --- Email codes are 6 digit codes sent to a provided email. Email c...(WORD_ESSAY_HYPHEN)
[style] ~35-~35: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...6 digit codes sent to a provided email. Email codes are enabled when transactional em...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
117-117: Add hyphen to compound adjective.When "full page" modifies the following noun "refresh", it should be hyphenated.
-try clearing your cookies & performing a full page refresh (`Cmd/Ctrl + Shift + R` on most browsers). +try clearing your cookies & performing a full-page refresh (`Cmd/Ctrl + Shift + R` on most browsers).🧰 Tools
🪛 LanguageTool
[uncategorized] ~117-~117: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ry clearing your cookies & performing a full page refresh (Cmd/Ctrl + Shift + Ron most...(EN_COMPOUND_ADJECTIVE_INTERNAL)
docs/self-hosting/upgrade/v3-to-v4-guide.mdx (1)
58-58: Fix grammar: "signing into" should be "signing in to".The phrase "signing into" is not standard English for computer systems. Use "signing in to" instead.
-If you're hitting issues with signing into your Sourcebot instance, make sure you're setting `AUTH_URL` correctly to your deployment domain (ex. `https://sourcebot.yourcompany.com`) +If you're hitting issues with signing in to your Sourcebot instance, make sure you're setting `AUTH_URL` correctly to your deployment domain (ex. `https://sourcebot.yourcompany.com`)🧰 Tools
🪛 LanguageTool
[grammar] ~58-~58: The verb ‘signing into’ is not standard English, except in the context of the law (“The bill was signed into law”). Write “signing in to”. For websites and computers, other options are “logging in to” or “logging on to”.
Context: ...hooting - If you're hitting issues with signing into your Sourcebot instance, make sure you'...(SIGN_INTO)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
docs/images/login.pngis excluded by!**/*.png
📒 Files selected for processing (6)
docs/docs/more/mcp-server.mdx(2 hunks)docs/self-hosting/configuration/authentication.mdx(1 hunks)docs/self-hosting/license-key.mdx(1 hunks)docs/self-hosting/upgrade/v3-to-v4-guide.mdx(1 hunks)packages/web/src/ee/sso/sso.tsx(1 hunks)packages/web/src/env.mjs(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- docs/self-hosting/license-key.mdx
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/docs/more/mcp-server.mdx
- packages/web/src/env.mjs
🧰 Additional context used
🪛 LanguageTool
docs/self-hosting/configuration/authentication.mdx
[grammar] ~22-~22: The word “setup” is a noun. The verb is spelled with a space.
Context: ... and approved on registration. You can setup emails to be sent when new join request...
(NOUN_VERB_CONFUSION)
[grammar] ~35-~35: When ‘6-digit’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...`. ### Email codes --- Email codes are 6 digit codes sent to a provided email. Email c...
(WORD_ESSAY_HYPHEN)
[style] ~35-~35: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...6 digit codes sent to a provided email. Email codes are enabled when transactional em...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[uncategorized] ~117-~117: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ry clearing your cookies & performing a full page refresh (Cmd/Ctrl + Shift + R on most...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
docs/self-hosting/upgrade/v3-to-v4-guide.mdx
[grammar] ~58-~58: The verb ‘signing into’ is not standard English, except in the context of the law (“The bill was signed into law”). Write “signing in to”. For websites and computers, other options are “logging in to” or “logging on to”.
Context: ...hooting - If you're hitting issues with signing into your Sourcebot instance, make sure you'...
(SIGN_INTO)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (1)
packages/web/src/ee/sso/sso.tsx (1)
122-132: Good fix for preventing duplicate memberships!The addition of the duplicate membership check properly addresses the previous review concern about creating duplicate userToOrg entries.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/web/src/ee/sso/sso.tsx (1)
92-165: JIT provisioning logic is well-implemented with previous concerns addressed.The function has been significantly improved:
- Error handling added via
sewwrapper ✓- Duplicate membership check added ✓
- Proper transaction handling ✓
- Seat limit enforcement ✓
However, consider making the
pendingApprovalupdate conditional based on the user's current approval status:await prisma.$transaction(async (tx) => { - await tx.user.update({ - where: { - id: userId, - }, - data: { - pendingApproval: false, - }, - }); + await tx.user.update({ + where: { + id: userId, + pendingApproval: true, + }, + data: { + pendingApproval: false, + }, + }); await tx.userToOrg.create({ data: { userId, orgId: org.id, role: OrgRole.MEMBER, }, }); });This ensures we only update users who were actually pending approval, making the operation more precise.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/web/src/ee/sso/sso.tsx(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/web/src/ee/sso/sso.tsx (3)
packages/web/src/actions.ts (1)
sew(47-55)packages/web/src/lib/serviceError.ts (1)
notFound(91-97)packages/web/src/features/entitlements/server.ts (2)
getSeats(63-66)SOURCEBOT_UNLIMITED_SEATS(8-8)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (2)
packages/web/src/ee/sso/sso.tsx (2)
1-16: LGTM!The imports are well-organized and include all necessary dependencies for SSO provider configuration and JIT provisioning functionality.
17-90: SSO provider configurations look correct.The function properly constructs OAuth providers based on environment variables. The GitHub provider configuration has been corrected from previous feedback, and all other providers follow standard configurations.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/web/src/app/[domain]/components/whatsNewIndicator.tsx (3)
20-35: Consider using a cookie library for more robust parsing.The manual cookie parsing implementation works but could be error-prone and is reinventing functionality available in established libraries. Consider using a library like
js-cookiefor more reliable cookie handling.+import Cookies from 'js-cookie' -const getReadItems = (): string[] => { - if (typeof document === "undefined") return [] - - const cookies = document.cookie.split(';').map(cookie => cookie.trim()) - const targetCookie = cookies.find(cookie => cookie.startsWith(`${COOKIE_NAME}=`)) - - if (!targetCookie) return [] - - try { - const cookieValue = targetCookie.substring(`${COOKIE_NAME}=`.length) - return JSON.parse(decodeURIComponent(cookieValue)) - } catch (error) { - console.warn('Failed to parse whats-new cookie:', error) - return [] - } -} +const getReadItems = (): string[] => { + if (typeof document === "undefined") return [] + + try { + const cookieValue = Cookies.get(COOKIE_NAME) + return cookieValue ? JSON.parse(cookieValue) : [] + } catch (error) { + console.warn('Failed to parse whats-new cookie:', error) + return [] + } +}
37-49: Consider shorter cookie expiration duration.A 1-year expiration for notification read state seems excessive. Consider using a shorter duration (e.g., 30-90 days) as user preferences for notifications may change more frequently.
- expires.setFullYear(expires.getFullYear() + 1) + expires.setTime(expires.getTime() + (90 * 24 * 60 * 60 * 1000)) // 90 days
89-95: Consider indicating external link behavior to users.The component opens external links in new tabs without explicit indication to users. Consider adding a visual indicator (like an external link icon) to set proper expectations.
+import { ExternalLink } from "lucide-react" // In the news item button: <div className="flex items-center gap-1 flex-shrink-0"> + <ExternalLink className="h-3 w-3 text-muted-foreground" /> {item.read ? ( <MailOpen className="h-3 w-3 text-muted-foreground group-hover:text-primary" /> ) : ( <Mail className="h-3 w-3 text-muted-foreground group-hover:text-primary" /> )} </div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/web/src/app/[domain]/components/navigationMenu.tsx(5 hunks)packages/web/src/app/[domain]/components/whatsNewIndicator.tsx(1 hunks)packages/web/src/lib/newsData.ts(1 hunks)packages/web/src/lib/types.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/web/src/lib/newsData.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/web/src/app/[domain]/components/navigationMenu.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (3)
packages/web/src/lib/types.ts (2)
14-17: Well-defined API key payload type.The
ApiKeyPayloadtype is clean and appropriately scoped for API key authentication. The two required string properties provide the necessary information for API key validation.
19-25: Clean news item type definition.The
NewsItemtype is well-structured with appropriate properties for a notification system. The optionalreadboolean property allows for flexible read state management.packages/web/src/app/[domain]/components/whatsNewIndicator.tsx (1)
51-179: Well-implemented notification component with good UX patterns.The component demonstrates excellent React patterns including proper SSR handling, cookie-based persistence, accessibility features, and responsive UI. The auto-mark as read functionality and unread count badge provide good user experience.
* Initial POC * Add cursor style rule * wip: resolve symbol definition in pop-up box * further wip * wip * wip: started work on bottom panel * wip: move find references to server action * wip * wip on fast readonly syntax highlighting using code mirror * refactor things around * Further wip: discovered that typical HighlightStyle.define highlighters cause performance problems when rendering thousands of code cells. The workaround found is to use tagHighlighter and rely on tailwind to drive theming in CSS. * wip: Add line number suppoert to the readonly code block * further wip: refactored how the bottom panel is rendered s.t., we don't need to re-render when doing a goto def * perf: memoize highlight ranges to prevent unecessary re-renders * chore: move search results list to use readonly ediotr * rename readOnlyCodeBlock to lightweightCodeHighlighter * improve syntax highlighting * nits with keyboard shortcuts * move match count aggregation into API layer * wip on adding definitions tab to bottom panel * Add eslint rule for tanstack query * Improve how we communicate multiple symbol definitions * Add support for revisions * chore: move stylized resize handle into shared component * chore: refactor code nav UI into ee * chore: made /browse the default path when clicking on a search result. More cleanup. * add code nav to preview panel * small bug fixes * wip: add scroll restoration to search results panel * small nit improvements * subtle changes to highlighting style * Use word boundaries for references * make bottom panel larger * fix visual artifact issues with filter panel * bidirectional filtering * debug code * Improve search heuristic by filtering by language * Improved collapsed preview panel style * add node types for a bunch of langs * nit: add tooltip to focus search bar shortcut hint * Add cmd+click to open preview panel * feedback * changelog * posthog events * wip on docs * docs and changelog * improve code nav search heuristics: search across repositories, and expand language filter to resolve related langs like TypeScript, JavaScript, TSX, and JSX. * delete unused /references route * improve search contexts news sub_header --------- Co-authored-by: msukkari <michael.sukkarieh@mail.mcgill.ca>
There was a problem hiding this comment.
Actionable comments posted: 8
🔭 Outside diff range comments (1)
packages/web/src/app/globals.css (1)
167-175:⚠️ Potential issueRemove duplicate CSS rules.
The
@layer baseblock and its contents are duplicated. The second occurrence (lines 220-228) should be removed.Apply this diff to remove the duplication:
-@layer base { - * { - @apply border-border; - } - - body { - @apply bg-background text-foreground; - } -}Also applies to: 220-228
♻️ Duplicate comments (1)
packages/web/src/lib/posthogEvents.ts (1)
254-274: Replace banned{}types to silence Biome warningsStatic analysis flags the new event payloads that use the banned
{}type. Stay consistent with the rest of the file while complying with the rule by switching toRecord<string, never>(zero-property object) which is semantically equivalent.- wa_requests_list_approve_request_success: {}, + wa_requests_list_approve_request_success: Record<string, never>, - wa_requests_list_reject_request_success: {}, + wa_requests_list_reject_request_success: Record<string, never>, - wa_api_key_created: {}, - wa_api_key_creation_fail: {}, + wa_api_key_created: Record<string, never>, + wa_api_key_creation_fail: Record<string, never>, - wa_preview_panel_find_references_pressed: {}, - wa_preview_panel_goto_definition_pressed: {}, + wa_preview_panel_find_references_pressed: Record<string, never>, + wa_preview_panel_goto_definition_pressed: Record<string, never>, - wa_browse_find_references_pressed: {}, - wa_browse_goto_definition_pressed: {}, + wa_browse_find_references_pressed: Record<string, never>, + wa_browse_goto_definition_pressed: Record<string, never>, - wa_explore_menu_reference_clicked: {}, + wa_explore_menu_reference_clicked: Record<string, never>,🧰 Tools
🪛 Biome (1.9.4)
[error] 254-254: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
[error] 259-259: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
[error] 264-264: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
[error] 265-265: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
[error] 267-267: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
[error] 268-268: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
[error] 270-270: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
[error] 271-271: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
[error] 273-273: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
🧹 Nitpick comments (16)
packages/web/src/app/[domain]/browse/[...path]/page.tsx (1)
124-138: Enhance accessibility for external link.The external link implementation is functional but could benefit from improved accessibility:
<a href={fileSourceResponse.webUrl} target="_blank" rel="noopener noreferrer" className="flex flex-row items-center gap-2 px-2 py-0.5 rounded-md flex-shrink-0" + aria-label={`Open ${path} in ${codeHostInfo.codeHostName}`} >packages/web/src/ee/features/codeNav/components/symbolHoverPopup/symbolHoverTargetsExtension.ts (1)
13-48: Comprehensive language support with potential for optimization.The node types list covers multiple programming languages (TypeScript, Python, C#, C/C++, Objective-C, Java, Rust, Go, PHP) which is excellent for broad language support. However, consider if this static list could be optimized based on the file's detected language.
Consider filtering the node types based on the file's language to improve performance:
+const getNodeTypesForLanguage = (language: string): string[] => { + const languageNodeTypes = { + typescript: ["VariableName", "VariableDefinition", "TypeDefinition", /*...*/], + python: ["VariableName", "VariableDefinition", "Identifier", /*...*/], + // ... other languages + }; + return languageNodeTypes[language.toLowerCase()] || NODE_TYPES; +}; + export const symbolHoverTargetsExtension = StateField.define<DecorationSet>({ create(state) { + const nodeTypes = getNodeTypesForLanguage(/* detect language from state */); // ... rest of implementation using nodeTypes instead of NODE_TYPESdocs/docs/search/code-navigation.mdx (1)
21-21: Fix grammar error in the documentation.Change "a inline preview" to "an inline preview" for correct grammar.
-| **Hover popover** | Hovering over a symbol reveals the symbol's definition signature as a inline preview. | +| **Hover popover** | Hovering over a symbol reveals the symbol's definition signature as an inline preview. |🧰 Tools
🪛 LanguageTool
[misspelling] ~21-~21: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...ls the symbol's definition signature as a inline preview. | | *Go to definition...(EN_A_VS_AN)
packages/web/src/app/[domain]/search/components/searchResultsPanel/fileMatchContainer.tsx (1)
126-147: Complex but correct match index calculation.The logic for calculating the match index when Ctrl+click is used is complex but appears correct. It accumulates the number of match ranges from all previous matches to determine the absolute match index.
Consider extracting this calculation into a helper function for better readability:
+ const calculateMatchIndex = (currentIndex: number) => { + return matches.slice(0, currentIndex).reduce((acc, match) => { + return acc + match.matchRanges.length; + }, 0); + }; onOpen={(startLineNumber, endLineNumber, isCtrlKeyPressed) => { if (isCtrlKeyPressed) { - const matchIndex = matches.slice(0, index).reduce((acc, match) => { - return acc + match.matchRanges.length; - }, 0); + const matchIndex = calculateMatchIndex(index); onOpenFilePreview(matchIndex);packages/web/src/app/[domain]/browse/browseStateProvider.tsx (1)
47-61: Good state hydration implementation with minor type safety suggestion.The implementation correctly handles parsing JSON state from query parameters and cleaning up the URL. The error handling prevents crashes from malformed JSON.
Consider adding type validation for the parsed state to ensure it conforms to the expected structure:
try { const parsedState = JSON.parse(hydratedBrowseState) as Partial<BrowseState>; + // Validate that parsedState has expected shape + if (typeof parsedState === 'object' && parsedState !== null) { onUpdateState(parsedState); + } } catch (error) { console.error("Error parsing hydratedBrowseState", error); }packages/web/src/hooks/useCodeMirrorTheme.ts (1)
59-133: Consider extracting the theme creation function.The
createThemeExtensionfunction is quite large (75 lines) and handles multiple concerns. Consider extracting it to a separate module for better maintainability and testability.// Create a new file: src/lib/codemirror/themeExtension.ts +export const createThemeExtension = ({ theme, settings = {} }: CreateThemeOptions): Extension => { + // Move the entire function implementation here +}; // Then import and use it: -const createThemeExtension = ({ theme, settings = {} }: CreateThemeOptions): Extension => { - // ... large function body -}; +import { createThemeExtension } from "@/lib/codemirror/themeExtension";packages/web/src/ee/features/codeNav/components/symbolHoverPopup/index.tsx (1)
80-92: Address the TODO comment for better UX.The TODO comment highlights an important UX improvement: requiring a modifier key (Ctrl/Cmd) to be held for navigation and showing visual feedback only when the key is pressed.
This would provide a more predictable user experience similar to IDEs like VS Code. The current implementation immediately navigates on click, which might be unexpected for users.
Do you want me to help implement the modifier key requirement and conditional underline styling?
packages/web/src/ee/features/codeNav/components/symbolHoverPopup/useHoveredOverSymbolInfo.ts (1)
48-50: Use optional chaining for cleaner null handling.The current pattern can be simplified using optional chaining as suggested by static analysis.
Apply this diff to improve readability:
- const symbolName = useMemo(() => { - return (symbolElement && symbolElement.textContent) ?? undefined; - }, [symbolElement]); + const symbolName = useMemo(() => { + return symbolElement?.textContent ?? undefined; + }, [symbolElement]);🧰 Tools
🪛 Biome (1.9.4)
[error] 49-49: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
packages/web/src/app/[domain]/browse/[...path]/components/codePreviewPanel.tsx (1)
202-202: Use optional chaining as suggested by static analysis.The condition can be simplified using optional chaining for better readability.
- {editorRef && editorRef.view && currentSelection && ( + {editorRef?.view && currentSelection && (🧰 Tools
🪛 Biome (1.9.4)
[error] 202-202: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
packages/web/src/app/[domain]/search/components/codePreviewPanel/index.tsx (1)
28-32: Simplify branch selection logic.The branch selection can be simplified using nullish coalescing.
- // If there are multiple branches pointing to the same revision of this file, it doesn't - // matter which branch we use here, so use the first one. - const branch = useMemo(() => { - return previewedFile.branches && previewedFile.branches.length > 0 ? previewedFile.branches[0] : undefined; - }, [previewedFile]); + // If there are multiple branches pointing to the same revision of this file, it doesn't + // matter which branch we use here, so use the first one. + const branch = useMemo(() => { + return previewedFile.branches?.[0]; + }, [previewedFile.branches]);packages/web/src/app/[domain]/search/components/codePreviewPanel/codePreview.tsx (1)
270-278: Consider adding error boundaries for the SymbolHoverPopup.Since this is an enterprise feature that depends on external services, consider wrapping it in an error boundary to prevent failures from affecting the entire preview.
Consider implementing an error boundary component specifically for enterprise features to gracefully handle failures in code navigation functionality without breaking the core preview experience.
packages/web/src/features/codeNav/actions.ts (1)
80-102: Remove unnecessary async from transform functionThe transform function doesn't perform any asynchronous operations, so the
asynckeyword is unnecessary.-const parser = searchResponseSchema.transform(async ({ files }) => ({ +const parser = searchResponseSchema.transform(({ files }) => ({packages/web/src/app/[domain]/search/components/filterPanel/index.tsx (2)
43-44: Consider optimizing filtering to reduce iterationsThe matches are being filtered twice separately. Consider creating a combined hook that computes both filtered sets in a single pass to improve performance, especially for large result sets.
You could create a combined hook like:
const { matchesFilteredByRepository, matchesFilteredByLanguage } = useFilteredMatchesCombined(matches);This would iterate through matches once and produce both filtered sets.
87-89: Simplify the counting logicThe
shouldCountcallback seems redundant with theisDisabledlogic. If an entry is not disabled, it means it exists in the filtered matches.Consider simplifying:
-if (!aggregation[key].isDisabled && shouldCount({ key, match })) { +if (!aggregation[key].isDisabled) { aggregation[key].count += 1; }Or if you need the additional filtering, document why both checks are necessary.
Also applies to: 208-210
packages/web/src/app/[domain]/search/page.tsx (2)
135-157: Consider documenting the distinction betweenmatchCountandtotalMatchCount.The code now uses both
searchResponse.stats.matchCount(assigned tomatchCount) andsearchResponse.zoektStats.matchCount(assigned tototalMatchCount). It would be helpful to add a comment explaining the difference between these two values for future maintainability.
263-287: Consider making the keyboard shortcut display OS-aware.The collapsed filter panel UI is well-designed. However, the shortcut hint always shows "⌘ B" which is Mac-specific. Consider detecting the OS and showing "Ctrl B" for Windows/Linux users.
- <KeyboardShortcutHint shortcut="⌘ B" /> + <KeyboardShortcutHint shortcut={navigator.platform.toLowerCase().includes('mac') ? "⌘ B" : "Ctrl B"} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
docs/images/demo.mp4is excluded by!**/*.mp4yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (64)
.cursor/rules/style.mdc(1 hunks)CHANGELOG.md(1 hunks)docs/docs.json(4 hunks)docs/docs/search/code-navigation.mdx(1 hunks)packages/web/.eslintrc.json(1 hunks)packages/web/package.json(5 hunks)packages/web/src/app/[domain]/browse/README.md(1 hunks)packages/web/src/app/[domain]/browse/[...path]/codePreview.tsx(0 hunks)packages/web/src/app/[domain]/browse/[...path]/components/codePreviewPanel.tsx(1 hunks)packages/web/src/app/[domain]/browse/[...path]/components/rangeHighlightingExtension.ts(1 hunks)packages/web/src/app/[domain]/browse/[...path]/page.tsx(4 hunks)packages/web/src/app/[domain]/browse/browseStateProvider.tsx(1 hunks)packages/web/src/app/[domain]/browse/components/bottomPanel.tsx(1 hunks)packages/web/src/app/[domain]/browse/hooks/useBrowseNavigation.ts(1 hunks)packages/web/src/app/[domain]/browse/hooks/useBrowseState.ts(1 hunks)packages/web/src/app/[domain]/browse/layout.tsx(1 hunks)packages/web/src/app/[domain]/components/connectionCreationForms/secretCombobox.tsx(1 hunks)packages/web/src/app/[domain]/components/editorContextMenu.tsx(2 hunks)packages/web/src/app/[domain]/components/fileHeader.tsx(3 hunks)packages/web/src/app/[domain]/components/keyboardShortcutHint.tsx(0 hunks)packages/web/src/app/[domain]/components/lightweightCodeHighlighter.tsx(1 hunks)packages/web/src/app/[domain]/components/searchBar/searchBar.tsx(3 hunks)packages/web/src/app/[domain]/components/searchBar/searchSuggestionsBox.tsx(1 hunks)packages/web/src/app/[domain]/components/searchBar/useSuggestionsData.ts(4 hunks)packages/web/src/app/[domain]/search/components/codePreviewPanel/codePreview.tsx(8 hunks)packages/web/src/app/[domain]/search/components/codePreviewPanel/index.tsx(1 hunks)packages/web/src/app/[domain]/search/components/filterPanel/entry.tsx(3 hunks)packages/web/src/app/[domain]/search/components/filterPanel/index.tsx(5 hunks)packages/web/src/app/[domain]/search/components/filterPanel/useFilterMatches.ts(1 hunks)packages/web/src/app/[domain]/search/components/filterPanel/useGetSelectedFromQuery.ts(1 hunks)packages/web/src/app/[domain]/search/components/searchResultsPanel/codePreview.tsx(0 hunks)packages/web/src/app/[domain]/search/components/searchResultsPanel/fileMatch.tsx(2 hunks)packages/web/src/app/[domain]/search/components/searchResultsPanel/fileMatchContainer.tsx(7 hunks)packages/web/src/app/[domain]/search/components/searchResultsPanel/index.tsx(4 hunks)packages/web/src/app/[domain]/search/components/searchResultsPanel/lightweightCodeMirror.tsx(0 hunks)packages/web/src/app/[domain]/search/page.tsx(10 hunks)packages/web/src/app/components/keyboardShortcutHint.tsx(1 hunks)packages/web/src/app/globals.css(4 hunks)packages/web/src/components/ui/animatedResizableHandle.tsx(1 hunks)packages/web/src/components/ui/loading-button.tsx(1 hunks)packages/web/src/ee/features/codeNav/components/exploreMenu/index.tsx(1 hunks)packages/web/src/ee/features/codeNav/components/exploreMenu/referenceList.tsx(1 hunks)packages/web/src/ee/features/codeNav/components/symbolHoverPopup/index.tsx(1 hunks)packages/web/src/ee/features/codeNav/components/symbolHoverPopup/symbolDefinitionPreview.tsx(1 hunks)packages/web/src/ee/features/codeNav/components/symbolHoverPopup/symbolHoverTargetsExtension.ts(1 hunks)packages/web/src/ee/features/codeNav/components/symbolHoverPopup/useHoveredOverSymbolInfo.ts(1 hunks)packages/web/src/features/codeNav/actions.ts(1 hunks)packages/web/src/features/codeNav/schemas.ts(1 hunks)packages/web/src/features/codeNav/types.ts(1 hunks)packages/web/src/features/entitlements/constants.ts(1 hunks)packages/web/src/features/search/fileSourceApi.ts(3 hunks)packages/web/src/features/search/schemas.ts(2 hunks)packages/web/src/features/search/searchApi.ts(7 hunks)packages/web/src/features/search/types.ts(1 hunks)packages/web/src/hooks/useCodeMirrorHighlighter.ts(1 hunks)packages/web/src/hooks/useCodeMirrorLanguageExtension.ts(1 hunks)packages/web/src/hooks/useCodeMirrorTheme.ts(1 hunks)packages/web/src/hooks/useTailwind.ts(0 hunks)packages/web/src/lib/extensions/searchResultHighlightExtension.ts(2 hunks)packages/web/src/lib/newsData.ts(1 hunks)packages/web/src/lib/posthogEvents.ts(1 hunks)packages/web/src/lib/utils.ts(6 hunks)packages/web/src/tailwind.ts(1 hunks)packages/web/tailwind.config.ts(1 hunks)
💤 Files with no reviewable changes (5)
- packages/web/src/hooks/useTailwind.ts
- packages/web/src/app/[domain]/components/keyboardShortcutHint.tsx
- packages/web/src/app/[domain]/search/components/searchResultsPanel/codePreview.tsx
- packages/web/src/app/[domain]/search/components/searchResultsPanel/lightweightCodeMirror.tsx
- packages/web/src/app/[domain]/browse/[...path]/codePreview.tsx
✅ Files skipped from review due to trivial changes (16)
- packages/web/src/app/[domain]/components/searchBar/searchSuggestionsBox.tsx
- packages/web/src/features/search/types.ts
- packages/web/src/app/components/keyboardShortcutHint.tsx
- .cursor/rules/style.mdc
- packages/web/src/app/[domain]/components/editorContextMenu.tsx
- packages/web/src/components/ui/animatedResizableHandle.tsx
- packages/web/src/app/[domain]/browse/README.md
- packages/web/src/features/search/schemas.ts
- packages/web/src/lib/newsData.ts
- packages/web/src/features/codeNav/types.ts
- CHANGELOG.md
- packages/web/src/hooks/useCodeMirrorLanguageExtension.ts
- packages/web/src/lib/extensions/searchResultHighlightExtension.ts
- packages/web/src/hooks/useCodeMirrorHighlighter.ts
- packages/web/src/app/[domain]/components/searchBar/useSuggestionsData.ts
- packages/web/src/tailwind.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/web/package.json
- docs/docs.json
- packages/web/src/features/entitlements/constants.ts
🧰 Additional context used
🧬 Code Graph Analysis (19)
packages/web/src/components/ui/loading-button.tsx (1)
packages/web/src/components/ui/button.tsx (2)
ButtonProps(36-40)Button(56-56)
packages/web/src/app/[domain]/components/searchBar/searchBar.tsx (2)
packages/web/src/components/ui/tooltip.tsx (3)
Tooltip(30-30)TooltipTrigger(30-30)TooltipContent(30-30)packages/web/src/app/components/keyboardShortcutHint.tsx (1)
KeyboardShortcutHint(8-22)
packages/web/src/features/codeNav/schemas.ts (1)
packages/web/src/features/search/schemas.ts (2)
rangeSchema(13-16)repositoryInfoSchema(34-40)
packages/web/src/ee/features/codeNav/components/symbolHoverPopup/symbolDefinitionPreview.tsx (4)
packages/web/src/features/search/types.ts (1)
SourceRange(29-29)packages/web/src/lib/utils.ts (1)
base64Decode(202-205)packages/web/src/components/ui/tooltip.tsx (3)
Tooltip(30-30)TooltipTrigger(30-30)TooltipContent(30-30)packages/web/src/app/[domain]/components/lightweightCodeHighlighter.tsx (1)
LightweightCodeHighlighter(38-149)
packages/web/src/app/[domain]/browse/[...path]/components/rangeHighlightingExtension.ts (1)
packages/web/src/app/[domain]/browse/hooks/useBrowseNavigation.ts (1)
BrowseHighlightRange(6-12)
packages/web/src/app/[domain]/search/components/filterPanel/useFilterMatches.ts (2)
packages/web/src/features/search/types.ts (1)
SearchResultFile(18-18)packages/web/src/app/[domain]/search/components/filterPanel/useGetSelectedFromQuery.ts (1)
useGetSelectedFromQuery(7-17)
packages/web/src/ee/features/codeNav/components/symbolHoverPopup/symbolHoverTargetsExtension.ts (1)
packages/web/src/lib/utils.ts (1)
measureSync(241-259)
packages/web/src/app/[domain]/components/fileHeader.tsx (1)
packages/web/src/app/[domain]/browse/hooks/useBrowseNavigation.ts (1)
useBrowseNavigation(25-59)
packages/web/src/app/[domain]/browse/browseStateProvider.tsx (2)
packages/web/src/app/[domain]/browse/components/bottomPanel.tsx (1)
BOTTOM_PANEL_MIN_SIZE(16-16)packages/web/src/hooks/useNonEmptyQueryParam.ts (1)
useNonEmptyQueryParam(18-25)
packages/web/src/ee/features/codeNav/components/symbolHoverPopup/index.tsx (5)
packages/web/src/ee/features/codeNav/components/symbolHoverPopup/useHoveredOverSymbolInfo.ts (2)
SymbolDefinition(17-23)useHoveredOverSymbolInfo(35-139)packages/web/src/ee/features/codeNav/components/symbolHoverPopup/symbolDefinitionPreview.tsx (1)
SymbolDefinitionPreview(18-62)packages/web/src/components/ui/separator.tsx (1)
Separator(31-31)packages/web/src/components/ui/loading-button.tsx (1)
LoadingButton(30-30)packages/web/src/components/ui/button.tsx (1)
Button(56-56)
packages/web/src/app/[domain]/browse/hooks/useBrowseNavigation.ts (1)
packages/web/src/app/[domain]/browse/browseStateProvider.tsx (2)
BrowseState(7-17)SET_BROWSE_STATE_QUERY_PARAM(26-26)
packages/web/src/ee/features/codeNav/components/symbolHoverPopup/useHoveredOverSymbolInfo.ts (4)
packages/web/src/features/search/types.ts (1)
SourceRange(29-29)packages/web/src/lib/utils.ts (1)
unwrapServiceError(289-296)packages/web/src/features/codeNav/actions.ts (1)
findSearchBasedSymbolDefinitions(47-77)packages/web/src/ee/features/codeNav/components/symbolHoverPopup/symbolHoverTargetsExtension.ts (1)
SYMBOL_HOVER_TARGET_DATA_ATTRIBUTE(6-6)
packages/web/src/hooks/useCodeMirrorTheme.ts (2)
packages/web/src/hooks/useThemeNormalized.ts (1)
useThemeNormalized(6-22)packages/web/src/hooks/useCodeMirrorHighlighter.ts (1)
useCodeMirrorHighlighter(6-86)
packages/web/src/app/[domain]/search/components/codePreviewPanel/codePreview.tsx (5)
packages/web/src/app/[domain]/browse/hooks/useBrowseNavigation.ts (1)
useBrowseNavigation(25-59)packages/web/src/features/entitlements/useHasEntitlement.ts (1)
useHasEntitlement(7-10)packages/web/src/hooks/useCodeMirrorLanguageExtension.ts (1)
useCodeMirrorLanguageExtension(7-18)packages/web/src/ee/features/codeNav/components/symbolHoverPopup/useHoveredOverSymbolInfo.ts (1)
SymbolDefinition(17-23)packages/web/src/ee/features/codeNav/components/symbolHoverPopup/index.tsx (1)
SymbolHoverPopup(19-138)
packages/web/src/app/[domain]/browse/[...path]/components/codePreviewPanel.tsx (13)
packages/web/src/hooks/useCodeMirrorLanguageExtension.ts (1)
useCodeMirrorLanguageExtension(7-18)packages/web/src/hooks/useKeymapExtension.ts (1)
useKeymapExtension(9-26)packages/web/src/features/entitlements/useHasEntitlement.ts (1)
useHasEntitlement(7-10)packages/web/src/app/[domain]/browse/hooks/useBrowseState.ts (1)
useBrowseState(6-12)packages/web/src/app/[domain]/browse/hooks/useBrowseNavigation.ts (3)
useBrowseNavigation(25-59)HIGHLIGHT_RANGE_QUERY_PARAM(14-14)BrowseHighlightRange(6-12)packages/web/src/hooks/useNonEmptyQueryParam.ts (1)
useNonEmptyQueryParam(18-25)packages/web/src/app/[domain]/browse/[...path]/components/rangeHighlightingExtension.ts (2)
update(35-37)rangeHighlightingExtension(15-39)packages/web/src/ee/features/codeNav/components/symbolHoverPopup/useHoveredOverSymbolInfo.ts (1)
SymbolDefinition(17-23)packages/web/src/hooks/useCodeMirrorTheme.ts (1)
useCodeMirrorTheme(14-55)packages/web/src/components/ui/resizable.tsx (1)
ResizablePanel(48-48)packages/web/src/components/ui/scroll-area.tsx (1)
ScrollArea(48-48)packages/web/src/app/[domain]/components/editorContextMenu.tsx (1)
EditorContextMenu(22-147)packages/web/src/ee/features/codeNav/components/symbolHoverPopup/index.tsx (1)
SymbolHoverPopup(19-138)
packages/web/src/features/search/fileSourceApi.ts (1)
packages/web/src/actions.ts (3)
sew(47-55)withAuth(57-110)withOrgMembership(141-189)
packages/web/src/app/[domain]/search/components/searchResultsPanel/index.tsx (1)
packages/web/src/features/search/types.ts (1)
SearchResultFile(18-18)
packages/web/src/app/[domain]/search/components/filterPanel/index.tsx (6)
packages/web/src/features/search/types.ts (2)
SearchResultFile(18-18)RepositoryInfo(28-28)packages/web/src/app/[domain]/search/components/filterPanel/useGetSelectedFromQuery.ts (1)
useGetSelectedFromQuery(7-17)packages/web/src/app/[domain]/search/components/filterPanel/useFilterMatches.ts (3)
useFilteredMatches(11-36)REPOS_QUERY_PARAM(8-8)LANGUAGES_QUERY_PARAM(7-7)packages/web/src/components/ui/fileIcon/fileIcon.tsx (1)
FileIcon(11-23)packages/web/src/app/[domain]/search/components/filterPanel/entry.tsx (2)
Entry(6-14)Entry(21-58)packages/web/src/app/[domain]/search/components/filterPanel/filter.tsx (1)
Filter(17-69)
packages/web/src/app/[domain]/search/page.tsx (9)
packages/web/src/hooks/useNonEmptyQueryParam.ts (1)
useNonEmptyQueryParam(18-25)packages/web/src/lib/utils.ts (2)
unwrapServiceError(289-296)createPathWithQueryParams(25-36)packages/web/src/features/search/types.ts (1)
SearchResultFile(18-18)packages/web/src/app/[domain]/search/components/filterPanel/useFilterMatches.ts (1)
useFilteredMatches(11-36)packages/web/src/components/ui/tooltip.tsx (3)
Tooltip(30-30)TooltipTrigger(30-30)TooltipContent(30-30)packages/web/src/components/ui/button.tsx (1)
Button(56-56)packages/web/src/app/components/keyboardShortcutHint.tsx (1)
KeyboardShortcutHint(8-22)packages/web/src/components/ui/animatedResizableHandle.tsx (1)
AnimatedResizableHandle(5-11)packages/web/src/app/[domain]/search/components/codePreviewPanel/index.tsx (1)
CodePreviewPanel(20-79)
🪛 LanguageTool
docs/docs/search/code-navigation.mdx
[misspelling] ~21-~21: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...ls the symbol's definition signature as a inline preview. | | *Go to definition...
(EN_A_VS_AN)
🪛 Biome (1.9.4)
packages/web/src/ee/features/codeNav/components/symbolHoverPopup/useHoveredOverSymbolInfo.ts
[error] 49-49: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
packages/web/src/app/[domain]/browse/[...path]/components/codePreviewPanel.tsx
[error] 202-202: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
packages/web/src/lib/posthogEvents.ts
[error] 254-254: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
[error] 259-259: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
[error] 264-264: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
[error] 265-265: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
[error] 267-267: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
[error] 268-268: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
[error] 270-270: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
[error] 271-271: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
[error] 273-273: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (84)
packages/web/src/app/[domain]/components/connectionCreationForms/secretCombobox.tsx (1)
43-43: LGTM! Proper domain-scoped query caching implemented.This change correctly updates the query key to include the domain, ensuring that secrets are cached per domain. This is essential for multi-tenancy support and prevents potential data leakage between different organization domains.
packages/web/src/app/[domain]/components/searchBar/searchBar.tsx (3)
45-46: LGTM: Import refactoring aligns with centralized Tailwind config approach.The changes correctly implement the broader refactoring mentioned in the AI summary:
KeyboardShortcutHintimport path updated to the new centralized location- Direct
tailwindconfig import replaces the previoususeTailwindhook patternThis approach provides better performance by avoiding hook overhead for static configuration data.
269-280: Excellent UX enhancement with accessible tooltip implementation.The addition of a tooltip around the keyboard shortcut hint significantly improves user experience by:
- Providing clear context ("Focus search bar") for the "/" shortcut
- Using appropriate delay (100ms) for responsive interaction
- Following consistent patterns with other tooltips in the component (like SearchHistoryButton)
- Maintaining accessibility with proper ARIA labeling from the underlying
KeyboardShortcutHintcomponentThe implementation correctly uses
TooltipTriggerwithasChildand wraps the hint in a div to ensure proper tooltip positioning.
163-163:✅ Verification successful
Verify theme dependency array optimization is correct.
The dependency array was changed to empty
[], which means the theme will only be computed once during component lifetime. This optimization is appropriate if thetailwindimport is now static rather than reactive.Please confirm that the
tailwindimport from@/tailwindprovides a static configuration object that doesn't change during runtime, making the empty dependency array safe.
🏁 Script executed:
#!/bin/bash # Description: Verify that the tailwind import provides static configuration # Expected: A static export that doesn't depend on React state or props cat packages/web/src/tailwind.tsLength of output: 212
To be certain the imported
tailwindConfigis a plain, static object, let’s inspect its source:
🏁 Script executed:
#!/bin/bash # Locate and print the project's Tailwind config to ensure it’s just a static export echo "Tailwind config file(s):" rg --files | grep -i "tailwind.config" echo "--- Content of tailwind.config ---" rg --files | grep -i "tailwind.config" | xargs -I{} sed -n '1,200p' {}Length of output: 4675
Theme import is static—empty dependency array is safe
The
tailwindexport is created once at module load by resolving a plain object frompackages/web/tailwind.config.ts, which contains only static values. There’s no runtime mutation or React state involved, so using[]as the dependency array insearchBar.tsxis safe.packages/web/src/components/ui/loading-button.tsx (3)
9-11: LGTM! Clean interface definition.The
LoadingButtonPropsinterface properly extendsButtonPropsand adds the optionalloadingboolean prop.
13-26: Well-implemented loading button component.The implementation correctly:
- Uses
forwardReffor proper ref forwarding- Combines
loadinganddisabledprops for the disabled state- Conditionally renders the loading spinner with appropriate styling
- Maintains all existing Button functionality
28-28: Good practice: Display name set for debugging.Setting the display name helps with React DevTools debugging and component identification.
packages/web/src/lib/utils.ts (5)
10-12: LGTM! Required imports for new utility function.The new imports support the
requiredQueryParamGuardfunction implementation with proper types and constants.
137-137: Good simplification of display name.Changing from "Generic Git Host" to "Git Host" makes the display more concise while maintaining clarity.
241-253: Excellent addition of optional logging control.The
outputLogparameter with a sensible default oftrueallows callers to control console output without breaking existing functionality.
261-273: Consistent implementation with measureSync.The async version maintains the same pattern and interface as the synchronous version for consistency.
298-308: Well-designed utility function for query parameter validation.The
requiredQueryParamGuardfunction:
- Uses proper TypeScript union return type
- Returns structured ServiceError with appropriate status code
- Provides clear error messaging
- Follows consistent error handling patterns
packages/web/src/app/[domain]/browse/hooks/useBrowseState.ts (1)
6-12: Well-implemented context hook with proper error handling.The hook correctly:
- Uses
useContextto access the BrowseStateContext- Includes defensive programming with a clear error message
- Follows standard React hook patterns for context access
- Provides good developer experience with meaningful error messaging
packages/web/.eslintrc.json (1)
10-11: Good addition of TanStack Query linting rules.Adding
"plugin:@tanstack/query/recommended"will enforce best practices for React Query usage across the codebase, which aligns with the new query-based data fetching implementations mentioned in the PR context.packages/web/src/app/[domain]/search/components/filterPanel/entry.tsx (2)
27-27: LGTM! Proper implementation of disabled state.The
isDisabledproperty is correctly destructured and applied to provide visual feedback with reduced opacity. This aligns well with the bidirectional filtering functionality described in the summary.Also applies to: 42-42
11-12:❌ Incorrect review comment
Clarify usage of
isHiddenproperty.The
isHiddenproperty is added to theEntrytype but is not used within the component logic. If this property is intended for parent components to conditionally render entries, consider adding a comment to clarify this design decision.
🏁 Script executed:
#!/bin/bash # Description: Check if isHidden property is used by parent components for conditional rendering # Search for usage of isHidden in filter panel related files rg -A 3 -B 3 "isHidden" packages/web/src/app/*/search/components/filterPanel/Length of output: 4887
Remove
isHiddenClarification RequestThe
isHiddenproperty is in fact consumed by the parentFilterPanelcomponent (index.tsx) to filter out hidden entries:
- Entries are tagged with
isHidden = isDisabled && !isSelectedwhen built.- The panel then does
const visibleRepos = useMemo(() => Object.values(repos).filter(entry => !entry.isHidden), [repos]); const visibleLanguages = useMemo(() => Object.values(languages).filter(entry => !entry.isHidden), [languages]);Because it’s already wired up for filtering, no additional comment or change is needed in
entry.tsx.Likely an incorrect or invalid review comment.
packages/web/src/app/[domain]/browse/layout.tsx (1)
1-26: Excellent layout design and architecture.This layout component demonstrates solid React patterns:
- Proper separation of concerns with
BrowseStateProvider- Clean vertical resizable panel structure
- Appropriate use of full-height container
- Well-organized component hierarchy
The integration with
BottomPanel,AnimatedResizableHandle, and state management aligns perfectly with the enhanced browsing experience described in the summary.packages/web/src/app/[domain]/search/components/filterPanel/useGetSelectedFromQuery.ts (1)
1-17: Well-implemented hook with proper optimization.The hook demonstrates excellent practices:
- Appropriate use of
useCallbackfor memoization- Clean handling of comma-separated values
- Proper fallback to empty
Setwhen parameter is absent- Type-safe implementation with clear return type
This will integrate seamlessly with the bidirectional filtering functionality.
packages/web/src/app/[domain]/browse/[...path]/page.tsx (4)
53-66: Excellent error handling improvement.The new error handling for repository not found provides a much better user experience compared to throwing exceptions. The UI feedback with icons and descriptive text is user-friendly and consistent with modern web application patterns.
98-103: Good integration of code host information.The extraction of code host info using the utility function is well-implemented and sets up the data needed for the external link feature.
106-149: Well-structured component layout with improved UX.The new layout structure with sticky top bar, proper separation of concerns, and integration with
CodePreviewPanelcreates a much more polished browsing experience. The file header integration and consistent styling demonstrate attention to detail.
80-81:❓ Verification inconclusive
Verify the branch defaulting behavior.
The change to default
branchto'HEAD'when no revision is specified needs verification to ensure it aligns with backend API expectations and doesn't cause issues with different git hosting providers.
🏁 Script executed:
#!/bin/bash # Description: Check how HEAD is handled in the backend API and other parts of the codebase # Search for HEAD usage in API routes and backend handling rg -A 5 -B 5 "HEAD.*branch|branch.*HEAD" packages/web/src/app/api/ rg -A 3 -B 3 "getFileSource" packages/web/src/features/search/Length of output: 1044
Verify default-branch fallback to “HEAD”
I didn’t find any handling for the literal
"HEAD"value in our API or search code—it's unclear whether the backend will accept and resolve"HEAD"as expected across all git hosts. Please confirm:
- The backend service resolves
"HEAD"to the repository’s default branch.- All supported Git providers accept
"HEAD"in the branch parameter.- No existing endpoints (e.g.
getFileSource, code-search APIs) break whenbranch = "HEAD".packages/web/src/features/codeNav/schemas.ts (2)
1-2: Good practice: Reusing existing schemas.The imports correctly reuse
rangeSchemaandrepositoryInfoSchemafrom the search schemas, promoting consistency across the codebase.
4-20: Schema structure looks comprehensive and well-typed.The schema properly defines the expected response structure for the find related symbols feature with:
- Statistics including match count
- File-level data with repository information and language metadata
- Match details with line content and range information
- Repository info array for additional context
The nested structure using array schemas and object schemas with proper validation is well-implemented.
packages/web/src/app/[domain]/browse/[...path]/components/rangeHighlightingExtension.ts (4)
7-13: Good separation of decoration types.The code properly defines separate decorations for mark-level (character-precise) and line-level highlighting with appropriate CSS classes.
26-33: Line-based highlighting logic is correct.The loop properly iterates through all lines in the range and applies line decorations using the correct line positioning from CodeMirror's document model.
35-38: Proper handling of document updates.The extension correctly maps decorations through transaction changes and provides them to the editor view, ensuring highlighting persists through document modifications.
19-25:✅ Verification successful
Potential off-by-one issue in column calculation.
The column calculation assumes 1-based indexing (
start.column - 1) but this needs verification against theBrowseHighlightRangetype definition to ensure consistency.Let me verify the column indexing used in the BrowseHighlightRange type:
🏁 Script executed:
#!/bin/bash # Description: Check if BrowseHighlightRange uses 0-based or 1-based column indexing # Expected: Find documentation or usage examples that clarify the indexing convention rg -A 10 -B 5 "BrowseHighlightRange" --type tsLength of output: 10187
🏁 Script executed:
#!/bin/bash # Show parsing logic for highlightRange in codePreviewPanel.tsx to determine column indexing rg -A 20 -B 5 "const highlightRange = useMemo" --type tsxLength of output: 87
🏁 Script executed:
#!/bin/bash # Locate and display the parsing logic for highlightRange in codePreviewPanel.tsx rg -A 30 -B 5 "const highlightRange" -g "*.tsx"Length of output: 11547
🏁 Script executed:
#!/bin/bash # Dump useBrowseNavigation hook to inspect highlightRange handling sed -n '1,200p' packages/web/src/app/*/browse/hooks/useBrowseNavigation.tsLength of output: 1884
No off-by-one in column conversion
Thecolumnvalues in BrowseHighlightRange are treated as 1-based (they’re serialized by navigateToPath and parsed back via parseInt). Subtracting 1 when computingstate.doc.line(lineNumber).from + column - 1correctly maps to CodeMirror’s 0-based offsets. No changes needed.
packages/web/src/app/[domain]/components/fileHeader.tsx (4)
1-1: Appropriate client-side directive.Adding
'use client'is necessary since the component now uses theuseBrowseNavigationhook which depends on browser APIs likeuseRouter.
8-8: Good integration with the navigation system.The import and usage of
useBrowseNavigationhook properly integrates the file header into the new browsing navigation system.Also applies to: 40-40
77-87: Navigation implementation looks correct.The click handler properly calls
navigateToPathwith appropriate parameters:
- Uses repo name and file name from props
- Sets path type to 'blob' for file viewing
- Uses branch display name as revision
- Adds cursor pointer and hover styles for better UX
The implementation aligns with the navigation hook's expected interface.
65-71: Minor styling improvements.The styling changes to the branch display simplify the markup and improve spacing consistency with explicit margin bottom and span wrapping.
packages/web/src/ee/features/codeNav/components/symbolHoverPopup/symbolHoverTargetsExtension.ts (5)
6-6: Good practice: Exporting the data attribute constant.Exporting
SYMBOL_HOVER_TARGET_DATA_ATTRIBUTEallows other components to reference the same attribute name, promoting consistency and reducing magic strings.
55-55: Good performance monitoring.Using
measureSyncto monitor syntax tree parsing performance is excellent practice, especially sinceensureSyntaxTreecan be expensive for large documents.
52-55: Correct use of ensureSyntaxTree for complete parsing.The comment correctly explains why
ensureSyntaxTreeis used instead ofsyntaxTree- to parse the entire document rather than just the visible viewport. This is necessary for comprehensive symbol detection.
64-72: Efficient tree iteration with proper decoration creation.The tree iteration logic is efficient and correctly creates decoration ranges only for matching node types. The commented debug code shows good development practices for troubleshooting.
74-77: Proper handling of document updates.The extension correctly maps decorations through transaction changes and provides them to the editor view, ensuring symbol targets remain correctly positioned during document edits.
packages/web/src/app/[domain]/search/components/filterPanel/useFilterMatches.ts (2)
11-36: LGTM! Well-structured filtering hook.The implementation is clean and follows React best practices:
- Proper use of
useMemofor performance optimization with correct dependencies- Clear separation of concerns with the
useGetSelectedFromQueryhook- Logical handling of empty filter sets (treats as "no filtering")
- Comprehensive switch statement covering all filter modes
The filtering logic correctly implements the expected behavior where empty sets mean "show all" rather than "show none".
7-8: Good use of exported constants for query parameters.Exporting the query parameter names as constants promotes consistency and reduces the risk of typos when these parameters are used elsewhere in the codebase.
packages/web/src/ee/features/codeNav/components/symbolHoverPopup/symbolDefinitionPreview.tsx (3)
30-50: Clarify the disabled tooltip trigger behavior.The
TooltipTriggeris set todisabled={true}but the tooltip still appears on hover. This might be intentional to prevent click interactions while maintaining hover functionality, but it could be confusing from a UX perspective.Please verify if this is the intended behavior or if the tooltip should be fully interactive.
24-26: Good use of memoization for base64 decoding.Properly memoizing the base64 decoding operation prevents unnecessary re-computation on each render, which is especially important for potentially large line content.
51-60: Well-configured code highlighter integration.The component properly configures the
LightweightCodeHighlighterwith appropriate props:
- Correct line number offset based on the source range
- Proper highlight ranges for the symbol
- Sensible defaults for line numbers and whitespace rendering
docs/docs/search/code-navigation.mdx (1)
28-44: Excellent technical documentation of search heuristics.The documentation clearly explains how the code navigation feature works under the hood, including:
- The search-based approach using existing search infrastructure
- Specific query patterns for finding references and definitions
- Explanation of the
sym:prefix for symbol definitions- Reference to universal ctags for context
This level of technical detail helps users understand the feature's capabilities and limitations.
packages/web/src/app/[domain]/search/components/searchResultsPanel/fileMatchContainer.tsx (2)
107-115: Good addition of dedicated Preview button.The new Preview button provides a clear way to open file previews without confusion about clickable areas. This improves the user experience by making the preview action explicit.
133-146: Well-implemented navigation integration.The integration with
useBrowseNavigationproperly handles direct navigation to files with highlight ranges. The conditional logic between Ctrl+click (preview) and normal click (navigation) provides good user control.packages/web/src/features/search/fileSourceApi.ts (3)
13-15: LGTM! Authentication refactor looks good.The function signature update to support API keys and the transition from session-based to userId-based authentication follows the established patterns from the
withAuthhelper inactions.ts. The minimum role requirement ofOrgRole.GUESTappropriately enables guest user access while maintaining security.
28-28: API key parameter properly propagated.Good implementation passing the
apiKeyparameter through to thesearchfunction, maintaining consistency with the authentication flow.
46-48: Enhanced response and proper role enforcement.The addition of
webUrlto the response enhances the API's utility. The explicit specification ofOrgRole.GUESTas the minimum required role and the conditional API key options are correctly implemented.packages/web/src/app/[domain]/browse/browseStateProvider.tsx (1)
7-24: Well-structured browse state interface.The
BrowseStateinterface and default state are well-designed for managing UI state across the browse experience. The use ofBOTTOM_PANEL_MIN_SIZEconstant ensures consistency with the bottom panel component.packages/web/src/app/[domain]/search/components/searchResultsPanel/fileMatch.tsx (3)
12-12: Enhanced callback signature for better interaction control.The updated
onOpenprop signature with line numbers and ctrl key state enables more sophisticated navigation behavior, allowing differentiation between direct file opening and preview modes.
25-30: Correct line number calculation logic.The implementation properly calculates start and end line numbers from the match content, accounting for the content start position and actual content length. The trimming prevents empty trailing lines from affecting the calculation.
53-61: Good replacement with LightweightCodeHighlighter.The transition from
CodePreviewtoLightweightCodeHighlighterwith explicit props for line numbers, offset, and whitespace rendering provides better control and likely improves performance. The highlight ranges are properly passed through.packages/web/src/app/[domain]/browse/hooks/useBrowseNavigation.ts (2)
6-12: Well-designed union type for highlight ranges.The
BrowseHighlightRangeunion type elegantly handles both line-only and line+column highlight scenarios. The discriminated union pattern makes it type-safe and clear when column information is available.
25-59: Hook implementation is robust and well-structured.The
useBrowseNavigationhook is implemented correctly with:
- Proper memoization using
useCallbackwith correct dependencies- Safe URL construction using
URLSearchParams- Clear parameter handling with sensible defaults
- Type-safe parameter destructuring
The highlight range encoding logic correctly handles both formats, and the JSON serialization for browse state is appropriate.
packages/web/src/hooks/useCodeMirrorTheme.ts (3)
5-12: Good refactor to remove external theme dependencies.The migration from
useTailwindand@uiw/codemirror-themesto direct tailwind config import and custom theme creation provides better control and reduces dependencies.
18-55: Theme creation logic is well-implemented.The hook correctly:
- Extracts theme colors from the tailwind config
- Uses proper memoization with appropriate dependencies
- Combines the custom theme extension with syntax highlighting
- Handles both light and dark themes
135-154: Well-defined TypeScript interfaces.The
CreateThemeOptionsandSettingsinterfaces provide good type safety for the theme creation function. The optional properties and proper typing ofStyleSpec['fontSize']demonstrate attention to type correctness.packages/web/src/app/[domain]/browse/components/bottomPanel.tsx (4)
19-34: Proper panel reference management.The component correctly uses
useReffor imperative panel control and synchronizes the panel state with the collapsed state viauseEffect. The effect properly handles both collapse and expand scenarios.
36-43: Well-implemented keyboard shortcuts.The keyboard shortcut implementation is robust:
- Uses proper key combination (shift+mod+e)
- Prevents default behavior
- Enables on form tags and contentEditable elements
- Includes descriptive text for accessibility
- Correctly toggles the panel state
78-93: ResizablePanel configuration is well-structured.The panel configuration properly:
- Sets appropriate size constraints (35-65%)
- Handles collapse/expand callbacks
- Updates state on resize only when not collapsed
- Uses proper order and ID for panel management
94-110: Excellent conditional rendering with entitlement checks.The component properly handles three states:
- No code-nav entitlement - shows informative message
- No symbol selected - shows helpful guidance
- Symbol selected - renders the ExploreMenu component
This provides a good user experience with clear feedback in all scenarios.
packages/web/src/ee/features/codeNav/components/symbolHoverPopup/index.tsx (3)
37-70: Sophisticated popup positioning implementation.The positioning logic using
@floating-ui/reactis well-implemented:
- Creates a virtual element from the symbol's bounding rect
- Uses appropriate middleware (offset, flip, shift) for smart positioning
- Properly constrains the popup within the editor bounds
- Updates position when symbol info or editor changes
83-92: Proper event listener cleanup.The effect correctly adds and removes the click event listener on the symbol element, ensuring no memory leaks when the component unmounts or symbol changes.
94-137: Excellent conditional rendering and user feedback.The component provides comprehensive user feedback:
- Loading state with spinner
- Symbol definition preview when available
- Appropriate messaging when no definitions found
- Disabled state for buttons when no definitions exist
- Dynamic button text based on definition count
The sticky behavior on mouse over/out prevents accidental popup dismissal during interaction.
packages/web/tailwind.config.ts (1)
1-153: Excellent Tailwind configuration restructuring to support new features.The comprehensive update to support detailed theming for code navigation, editor syntax highlighting, and UI animations is well-organized and follows Tailwind best practices. The new color tokens for
sidebarandeditorwith extensive syntax highlighting tags, along with the keyframe animations, properly support the code navigation features being introduced.packages/web/src/ee/features/codeNav/components/exploreMenu/referenceList.tsx (1)
1-117: Well-implemented component following React best practices.The
ReferenceListcomponent demonstrates excellent structure with:
- Proper memoization of repository info mapping for performance
- Correct sorting of matches by line number
- Clean integration with navigation hooks and event capture
- Appropriate base64 decoding and syntax highlighting
- Good separation of concerns between list and item components
The implementation is maintainable and follows established patterns for code navigation features.
packages/web/src/ee/features/codeNav/components/exploreMenu/index.tsx (1)
1-205: Excellent implementation of code navigation UI with proper state management.The
ExploreMenucomponent demonstrates solid architecture with:
- Proper React Query usage for parallel data fetching with appropriate error and loading states
- Clean resizable panel layout with good UX considerations
- Robust count formatting logic that handles edge cases (undefined, >999)
- Well-integrated tab state management with global browse state
- Appropriate fallback UI for empty states
The component handles complex UI interactions while maintaining clean separation of concerns.
packages/web/src/ee/features/codeNav/components/symbolHoverPopup/useHoveredOverSymbolInfo.ts (1)
35-139: Well-implemented custom hook with proper resource management.The
useHoveredOverSymbolInfohook demonstrates excellent practices:
- Proper timer management with cleanup to prevent memory leaks
- Correct event listener lifecycle management
- Appropriate React Query configuration with proper caching (
staleTime: Infinity)- Good use of memoization for performance optimization
- Clean state management with proper conditional returns
The hover delay timings (500ms show, 100ms hide) provide good UX balance.
🧰 Tools
🪛 Biome (1.9.4)
[error] 49-49: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
packages/web/src/app/[domain]/browse/[...path]/components/codePreviewPanel.tsx (1)
117-134:❓ Verification inconclusive
Consider edge cases in scroll behavior.
The scrolling logic should handle cases where the highlighted range extends beyond the document bounds.
🏁 Script executed:
#!/bin/bash # Check if there are any existing patterns for handling out-of-bounds line numbers in CodeMirror usage rg -A 5 "doc\.line\(" --glob "*.tsx" --glob "*.ts" | grep -B 2 -A 2 "try\|catch\|Math\.(min|max)"Length of output: 99
Ensure scroll logic handles out-of-bounds line numbers
We didn’t find any existing boundary checks around your
doc.line(…)calls. IfhighlightRange.lineNumberis less than 1 or greater than the document’s total lines,doc.line()could throw or produce unexpected results. Consider clamping or validating the range before scrolling. For example:useEffect(() => { - const { start, end } = highlightRange; - const selection = EditorSelection.range( - doc.line(start.lineNumber).from, - doc.line(end.lineNumber).from, - ); + const { start, end } = highlightRange; + const totalLines = doc.lines; // clamp to valid range + const clamp = (n: number) => Math.min(Math.max(n, 1), totalLines); + const fromLine = doc.line(clamp(start.lineNumber)).from; + const toLine = doc.line(clamp(end.lineNumber)).from; + const selection = EditorSelection.range(fromLine, toLine); editorRef.view?.dispatch({ effects: [ EditorView.scrollIntoView(selection, { y: "center" }), ] }); }, [editorRef, highlightRange]);• File:
packages/web/src/app/[domain]/browse/[...path]/components/codePreviewPanel.tsx
• Lines: 117–134Please verify that
doc.lines(or equivalent) correctly reflects the document’s line count in your CodeMirror setup, or wrap the calls in atry/catchif you prefer exception handling.packages/web/src/app/[domain]/search/components/codePreviewPanel/codePreview.tsx (1)
193-204: LGTM! Good UX improvement.The change from external link to internal navigation provides a more seamless browsing experience.
packages/web/src/features/search/searchApi.ts (3)
127-134: LGTM! Enhanced authentication support.The function signature changes properly implement API key authentication and user ID-based access control, aligning with the PR's authentication overhaul objectives.
235-241: Good defensive programming!The null check and Sentry logging for missing repositories prevents runtime errors and provides visibility into data inconsistencies.
334-343: Efficient match count aggregation.The nested reduce operations correctly calculate the total match count across all files and chunks. This provides useful statistics for search results.
packages/web/src/app/[domain]/components/lightweightCodeHighlighter.tsx (1)
38-149: Well-implemented lightweight code highlighter!The component is properly memoized and handles async highlighting efficiently. Good use of React patterns and performance optimizations.
packages/web/src/app/globals.css (2)
187-199: Modern and accessible search highlighting.Excellent use of
color-mix()for transparent overlays and distinct visual styles for selected vs non-selected matches.
43-86: Comprehensive editor theming system!The extensive set of CSS variables provides excellent theming support for both light and dark modes, covering all syntax highlighting needs.
Also applies to: 124-164
packages/web/src/features/codeNav/actions.ts (1)
107-118: LGTM!Good approach to group related language variants together for more comprehensive search results.
packages/web/src/app/[domain]/search/components/searchResultsPanel/index.tsx (1)
76-116: Excellent scroll state management implementation!The implementation shows great attention to UX details:
- Proper debouncing to avoid excessive history updates
- Smart scroll-to-top behavior when collapsing matches
- Clean state reset when file matches change
packages/web/src/app/[domain]/search/page.tsx (6)
25-33: LGTM! Well-organized imports for the new features.All the imported modules are properly used throughout the component for filter panel management, keyboard shortcuts, and UI enhancements.
35-35: Good naming improvement for clarity.The rename from
DEFAULT_MATCH_COUNTtoDEFAULT_MAX_MATCH_COUNTbetter conveys that this represents an upper limit for search results.
56-58: Verify the query parameter name consistency.The code still uses
SearchQueryParams.matcheswhile the AI summary indicates this should be updated tomaxMatchCount. Please ensure the query parameter name is consistent throughout the codebase.#!/bin/bash # Description: Check for SearchQueryParams definition and usage of 'matches' vs 'maxMatchCount' parameter # Find the SearchQueryParams definition echo "=== SearchQueryParams definition ===" rg -A 10 "SearchQueryParams\s*=" --type typescript # Check for usage of matches query parameter echo -e "\n=== Usage of 'matches' query parameter ===" rg "SearchQueryParams\.matches" --type typescript # Check for any 'maxMatchCount' query parameter usage echo -e "\n=== Usage of 'maxMatchCount' query parameter ===" rg "maxMatchCount.*query|query.*maxMatchCount" --type typescriptLikely an incorrect or invalid review comment.
222-227: Excellent state management improvements!The changes improve code clarity and user experience:
previewedFileis more semantically accurate thanselectedFile- Extracting filtering logic to
useFilteredMatchesfollows separation of concerns- Persisting the filter panel state enhances user experience across sessions
229-239: Well-implemented keyboard shortcut!The Cmd/Ctrl+B shortcut for toggling the filter panel is intuitive and follows common UI patterns. Good decision to enable it on form elements for better accessibility.
333-352: Clean implementation of conditional preview panel rendering!The code properly handles the preview panel lifecycle:
- Only renders when a file is selected for preview
- Clears the preview state when panel is collapsed
- Maintains UI consistency with animated handles
packages/web/src/app/[domain]/browse/[...path]/components/codePreviewPanel.tsx
Show resolved
Hide resolved
packages/web/src/app/[domain]/browse/[...path]/components/codePreviewPanel.tsx
Show resolved
Hide resolved
packages/web/src/app/[domain]/search/components/codePreviewPanel/index.tsx
Show resolved
Hide resolved
packages/web/src/app/[domain]/search/components/codePreviewPanel/codePreview.tsx
Show resolved
Hide resolved
packages/web/src/app/[domain]/search/components/searchResultsPanel/index.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
docs/self-hosting/upgrade/v3-to-v4-guide.mdx (1)
14-14: Heading simplification applied
The heading now reads “If your deployment doesn’t have authentication enabled,” removing theSOURCEBOT_AUTH_ENABLEDreference. This matches earlier feedback.
🧹 Nitpick comments (10)
docs/docs/search/code-navigation.mdx (4)
21-21: Typographical correction: Use 'an' before vowel sounds
The phrase reads "as a inline preview" but should be "as an inline preview" for grammatical correctness.- Hovering over a symbol reveals the symbol's definition signature as a inline preview. + Hovering over a symbol reveals the symbol's definition signature as an inline preview.🧰 Tools
🪛 LanguageTool
[misspelling] ~21-~21: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...ls the symbol's definition signature as a inline preview. | | *Go to definition...(EN_A_VS_AN)
9-9: Fix external link protocol
The link[Sourcebot cloud](app.sourcebot.dev)is missing the scheme and will be treated as an internal link. It should point tohttps://app.sourcebot.dev.- This feature is only available in [Sourcebot cloud](app.sourcebot.dev) + This feature is only available in [Sourcebot cloud](https://app.sourcebot.dev)
34-34: Simplify regex escape syntax in code block
In fenced code blocks, backslashes are literal. You only need single backslashes (\b) to represent word boundaries.- \\b{symbolName}\\b rev:{git_revision} lang:{language} case:yes + \b{symbolName}\b rev:{git_revision} lang:{language} case:yes
41-41: Simplify regex escape syntax in code block
Similarly, update the definition search to use single backslashes for word boundaries.- sym:\\b{symbolName}\\b rev:{git_revision} lang:{language} + sym:\b{symbolName}\b rev:{git_revision} lang:{language}CHANGELOG.md (1)
10-11: Refine wording and punctuation in introduction.
Consider adding a comma for list clarity and using the imperative "Check out" for consistency.
Apply this diff:- Sourcebot V4 introduces authentication, performance improvements and code navigation. Checkout the [migration guide](https://docs.sourcebot.dev/self-hosting/upgrade/v3-to-v4-guide) for information on upgrading your instance to v4. + Sourcebot V4 introduces authentication, performance improvements, and code navigation. Check out the [migration guide](https://docs.sourcebot.dev/self-hosting/upgrade/v3-to-v4-guide) for information on upgrading your instance to v4.🧰 Tools
🪛 LanguageTool
[grammar] ~10-~10: This sentence should probably be started with a verb instead of the noun ‘Checkout’. If not, consider inserting a comma for better clarity.
Context: ...mance improvements and code navigation. Checkout the [migration guide](https://docs.sour...(SENT_START_NN_DT)
docs/docs/search/search-contexts.mdx (1)
19-19: Verify updated declarative-config link & style nitpick
The link to the declarative configuration now correctly points to/self-hosting/configuration/declarative-config. Consider tightening the phrase “inside of a” to “inside” for readability:-Search contexts are defined in the `context` object inside of a [declarative config](...) +Search contexts are defined in the `context` object inside a [declarative config](...)🧰 Tools
🪛 LanguageTool
[style] ~19-~19: This phrase is redundant. Consider using “inside”.
Context: ...xts are defined in thecontextobject inside of a [declarative config](/self-hosting/co...(OUTSIDE_OF)
docs/self-hosting/upgrade/v3-to-v4-guide.mdx (4)
1-4: Consistent casing for sidebarTitle
ThesidebarTitleuses lowercase “guide” while thetitlecapitalizes “Guide”. For consistency in the sidebar navigation, consider updatingsidebarTitle: V3 to V4 Guide.
8-12: Refine enterprise license phrasing
In the Warning block, the sentence on unauthenticated access reads a bit clunky. Consider rephrasing for clarity, e.g.:-Unauthenticated access to a Sourcebot deployment can be enabled with an unlimited seat [enterprise license](/self-hosting/license-key) +With an unlimited-seat [Enterprise license](/self-hosting/license-key), you can enable unauthenticated access to your organization.
49-49: Add comma for readability
Insert a comma after “organization” to improve flow:-New user registrations will now submit a request to join the organization which can be approved… +New user registrations will now submit a request to join the organization, which can be approved…🧰 Tools
🪛 LanguageTool
[uncategorized] ~49-~49: Possible missing comma found.
Context: ...s will now submit a request to join the organization which can be approved by the owner by ...(AI_HYDRA_LEO_MISSING_COMMA)
58-58: Grammar: “signing into” → “signing in to”
Update the phrase for standard usage:-If you're hitting issues with signing into your Sourcebot instance… +If you're hitting issues with signing in to your Sourcebot instance…🧰 Tools
🪛 LanguageTool
[grammar] ~58-~58: The verb ‘signing into’ is not standard English, except in the context of the law (“The bill was signed into law”). Write “signing in to”. For websites and computers, other options are “logging in to” or “logging on to”.
Context: ...hooting - If you're hitting issues with signing into your Sourcebot instance, make sure you'...(SIGN_INTO)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
CHANGELOG.md(2 hunks)docs/docs.json(4 hunks)docs/docs/connections/gitea.mdx(2 hunks)docs/docs/connections/github.mdx(2 hunks)docs/docs/connections/gitlab.mdx(2 hunks)docs/docs/connections/overview.mdx(1 hunks)docs/docs/search/code-navigation.mdx(1 hunks)docs/docs/search/search-contexts.mdx(3 hunks)docs/self-hosting/configuration/tenancy.mdx(1 hunks)docs/self-hosting/upgrade/v3-to-v4-guide.mdx(1 hunks)docs/snippets/bitbucket-app-password.mdx(2 hunks)docs/snippets/bitbucket-token.mdx(2 hunks)packages/mcp/CHANGELOG.md(1 hunks)packages/web/src/app/[domain]/browse/components/bottomPanel.tsx(1 hunks)packages/web/src/features/codeNav/actions.ts(1 hunks)packages/web/src/features/entitlements/constants.ts(1 hunks)
✅ Files skipped from review due to trivial changes (8)
- docs/self-hosting/configuration/tenancy.mdx
- docs/docs/connections/github.mdx
- docs/docs/connections/overview.mdx
- docs/docs/connections/gitea.mdx
- docs/snippets/bitbucket-token.mdx
- packages/mcp/CHANGELOG.md
- docs/docs/connections/gitlab.mdx
- docs/snippets/bitbucket-app-password.mdx
🚧 Files skipped from review as they are similar to previous changes (4)
- docs/docs.json
- packages/web/src/app/[domain]/browse/components/bottomPanel.tsx
- packages/web/src/features/entitlements/constants.ts
- packages/web/src/features/codeNav/actions.ts
🧰 Additional context used
🪛 LanguageTool
CHANGELOG.md
[grammar] ~10-~10: This sentence should probably be started with a verb instead of the noun ‘Checkout’. If not, consider inserting a comma for better clarity.
Context: ...mance improvements and code navigation. Checkout the [migration guide](https://docs.sour...
(SENT_START_NN_DT)
docs/self-hosting/upgrade/v3-to-v4-guide.mdx
[uncategorized] ~49-~49: Possible missing comma found.
Context: ...s will now submit a request to join the organization which can be approved by the owner by ...
(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~58-~58: The verb ‘signing into’ is not standard English, except in the context of the law (“The bill was signed into law”). Write “signing in to”. For websites and computers, other options are “logging in to” or “logging on to”.
Context: ...hooting - If you're hitting issues with signing into your Sourcebot instance, make sure you'...
(SIGN_INTO)
docs/docs/search/code-navigation.mdx
[misspelling] ~21-~21: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...ls the symbol's definition signature as a inline preview. | | *Go to definition...
(EN_A_VS_AN)
docs/docs/search/search-contexts.mdx
[style] ~19-~19: This phrase is redundant. Consider using “inside”.
Context: ...xts are defined in the context object inside of a [declarative config](/self-hosting/co...
(OUTSIDE_OF)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (5)
CHANGELOG.md (2)
12-20: Expand "Changed" section to cover seat enforcement and public access.
The PR introduces mandatory authentication workflows, license seat count enforcement, public access controls, guest user creation, and owner approval flows. Please update this section to reflect:
- Enforcement of enterprise license seat limits when adding new members
- Optional public access toggle for non-authenticated users under an EE license
- Guest user role assignment and entitlements
Likely an incorrect or invalid review comment.
25-27: Fixed section entry is accurate.
The bullet for improved scroll performance aligns with the implemented fix and is clear.docs/docs/search/search-contexts.mdx (2)
3-3: Removal of "(EE)" suffix is correct
The sidebar title no longer includes the "(EE)" suffix, aligning with the updated documentation structure. Good to merge.
44-44: Link to declarative-config in example is accurate
The example now references the correct path/self-hosting/configuration/declarative-config. No further changes needed.docs/self-hosting/upgrade/v3-to-v4-guide.mdx (1)
54-56: Internal link fragment is correct
The link to[instructions above](/self-hosting/upgrade/v3-to-v4-guide#if-your-deployment-doesnt-have-authentication-enabled)now matches the actual section ID. Perfect.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
CHANGELOG.md (1)
24-24: Include documentation link for API key management.
A direct link to the API key docs will help users find details quickly.Suggested diff:
- Added Sourcebot API key management for external clients. [#311](https://github.com/sourcebot-dev/sourcebot/pull/311) + Added Sourcebot API key management for external clients. [Read the documentation](https://docs.sourcebot.dev/docs/auth/api-keys). [#311](https://github.com/sourcebot-dev/sourcebot/pull/311)
🧹 Nitpick comments (2)
CHANGELOG.md (2)
10-10: Improve imperative form and grammar in description.
The sentence starts with the noun “Checkout”—consider using the verb form “Check out” for clarity.Apply this diff:
- Sourcebot V4 introduces authentication, performance improvements and code navigation. Checkout the [migration guide](https://docs/sourcebot.dev/self-hosting/upgrade/v3-to-v4-guide) for information on upgrading your instance to v4. + Sourcebot V4 introduces authentication, performance improvements and code navigation. Check out the [migration guide](https://docs.sourcebot.dev/self-hosting/upgrade/v3-to-v4-guide) for information on upgrading your instance to v4.🧰 Tools
🪛 LanguageTool
[grammar] ~10-~10: This sentence should probably be started with a verb instead of the noun ‘Checkout’. If not, consider inserting a comma for better clarity.
Context: ...mance improvements and code navigation. Checkout the [migration guide](https://docs.sour...(SENT_START_NN_DT)
115-116: Fix article usage and hyphenation.
Change “a integrated web-editor” to “an integrated web editor” and remove the unnecessary hyphen for improved readability.- - connection management: create and manage your JSON configs via a integrated web-editor. + - connection management: create and manage your JSON configs via an integrated web editor.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
CHANGELOG.md(2 hunks)
🧰 Additional context used
🪛 LanguageTool
CHANGELOG.md
[grammar] ~10-~10: This sentence should probably be started with a verb instead of the noun ‘Checkout’. If not, consider inserting a comma for better clarity.
Context: ...mance improvements and code navigation. Checkout the [migration guide](https://docs.sour...
(SENT_START_NN_DT)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (5)
CHANGELOG.md (5)
12-12: The “### Changed” section heading follows the project's changelog conventions; no update needed.
13-18: Clear description of breaking changes and workflow.
The bullet points under "Changed" succinctly outline the new authentication requirements, role assignments, and configuration options, with appropriate links to detailed docs.
19-19: The description of the new search-result navigation behavior is clear and consistent with the rest of the "Changed" section.
21-21: The “### Added” section heading adheres to the project's changelog style; no changes needed.
22-23: Well-documented feature additions.
The new entries for EE code navigation and the collapsible filter panel are clearly stated with appropriate links and PR references.
Sourcebot V4 introduces authentication, performance improvements and code navigation. Checkout the migration guide for information on upgrading your instance to v4.
Changed
ownerrole. (docs).ownercan approve / deny requests to join the instance viaSettings>Members>Pending Requests.memberrole./browseview. Files can still be previewed by clicking the "Preview" button or holdingCmd/Ctrlwhen clicking on a search result. #315Added
Fixed
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Documentation
Style
Chores