My docs cleanup #4519

pablonyx · 2025-04-13T23:07:29Z

Description

Fixes https://linear.app/danswer/issue/DAN-1907/get-my-docs-cleanup-in

How Has This Been Tested?

[Describe the tests you ran to verify your changes]

Backporting (check the box to trigger backport action)

Note: You have to check that the action passes, otherwise resolve the conflicts manually and tag the patches.

This PR should be backported (make sure to check that the backport attempt succeeds)
[Optional] Override Linear Check

vercel · 2025-04-13T23:07:33Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
internal-search	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Apr 25, 2025 4:30am

greptile-apps

PR Summary

Based on the provided changes, here is a concise summary of the key modifications in this pull request:

The PR primarily focuses on cleanup and UI improvements across the codebase, with some functional changes to persona management and tenant provisioning.

Key changes:

Improved tenant provisioning by renaming Redis lock to CLOUD_PRE_PROVISION_TENANT_LOCK and adding proper cloud task scheduling
Enhanced persona privacy management by adding creator_user_id parameter to prevent unnecessary notifications
Streamlined file/folder management in MyDocuments components by removing unused move/rename functionality and improving mobile responsiveness
Added dark mode support and mobile-friendly styling across multiple components
Fixed potential redirect loops in authentication flow by adding source tracking

The changes appear focused on improving code organization and user experience while maintaining core functionality. However, some areas need attention:

The removal of forgot password functionality in cloud mode needs documentation explaining the rationale
The LaTeX content processing changes should be tested thoroughly
The Modal overflow change from 'auto' to 'visible' could cause layout issues

_{56 file(s) reviewed, 29 comment(s)}
_{Edit PR Review Bot Settings | Greptile}

greptile-apps · 2025-04-25T04:21:04Z

backend/ee/onyx/db/persona.py

+            if user_id != creator_user_id:
+                create_notification(
+                    user_id=user_id,
+                    notif_type=NotificationType.PERSONA_SHARED,
+                    db_session=db_session,
+                    additional_data=PersonaSharedNotificationData(
+                        persona_id=persona_id,
+                    ).model_dump(),
+                )


logic: Notification check correctly prevents sending to creator, but should also check if creator_user_id is None to avoid potential false negatives

Suggested change

if user_id != creator_user_id:

create_notification(

user_id=user_id,

notif_type=NotificationType.PERSONA_SHARED,

db_session=db_session,

additional_data=PersonaSharedNotificationData(

persona_id=persona_id,

).model_dump(),

)

if creator_user_id is not None and user_id != creator_user_id:

create_notification(

user_id=user_id,

notif_type=NotificationType.PERSONA_SHARED,

db_session=db_session,

additional_data=PersonaSharedNotificationData(

persona_id=persona_id,

).model_dump(),

)

greptile-apps · 2025-04-25T04:21:55Z

backend/onyx/configs/constants.py

    MONITOR_BACKGROUND_PROCESSES_LOCK = "da_lock:monitor_background_processes"
    CHECK_AVAILABLE_TENANTS_LOCK = "da_lock:check_available_tenants"
-    PRE_PROVISION_TENANT_LOCK = "da_lock:pre_provision_tenant"
+    CLOUD_PRE_PROVISION_TENANT_LOCK = "da_lock:pre_provision_tenant"


style: The lock name string value remains unchanged despite the constant being renamed to include 'CLOUD_'. Consider updating the string value to also include 'cloud_' for consistency.

greptile-apps · 2025-04-25T04:22:31Z

backend/onyx/server/user_documents/api.py

+    folders = (
+        db_session.query(UserFolder)
+        .filter(
+            (UserFolder.user_id == user_id) | (UserFolder.id == RECENT_DOCS_FOLDER_ID)


logic: Potential security issue: RECENT_DOCS_FOLDER_ID is used before being defined (line 144). Move constant definition to top of file.

greptile-apps · 2025-04-25T04:22:32Z

backend/onyx/server/user_documents/api.py

+        folder_snapshot.files = [
+            file for file in folder_snapshot.files if file.user_id == user_id
+        ]


style: Performance concern: filtering files in memory instead of in the database query. Consider adding user_id filter to the initial query.

greptile-apps · 2025-04-25T04:22:38Z

backend/shared_configs/contextvars.py

 def get_current_tenant_id() -> str:
    tenant_id = CURRENT_TENANT_ID_CONTEXTVAR.get()
    if tenant_id is None:
+        import traceback


style: importing traceback inside function scope - consider moving to top-level imports for better performance and readability

greptile-apps · 2025-04-25T04:29:32Z

web/src/components/TokenDisplay.tsx

+              <div className="hidden sm:block relative w-24 h-2 bg-neutral-200 dark:bg-neutral-700 rounded-full overflow-hidden">
                <div
-                  className={`absolute top-0 left-0 h-full rounded-full ${
+                  className={` absolute top-0 left-0 h-full rounded-full ${


style: Extra space at start of className template literal is unnecessary

greptile-apps · 2025-04-25T04:29:49Z

web/src/components/chat/TextView.tsx

+  // Log render count on each render
+  useEffect(() => {
+    setRenderCount((prevCount) => prevCount + 1);
+    console.log(`TextView component rendered ${renderCount + 1} times`);
+  }, []);


logic: This effect creates unnecessary re-renders and console spam. Remove the render count tracking as it serves no production purpose and impacts performance.

Suggested change

// Log render count on each render

useEffect(() => {

setRenderCount((prevCount) => prevCount + 1);

console.log(`TextView component rendered ${renderCount + 1} times`);

}, []);

greptile-apps · 2025-04-25T04:29:49Z

web/src/components/chat/TextView.tsx

  useEffect(() => {
    fetchFile();
-  }, [fetchFile]);
+  }, []);


logic: Empty dependency array but using fetchFile callback - this will use stale closure. Should include [fetchFile] in dependencies.

Suggested change

useEffect(() => {

fetchFile();

}, [fetchFile]);

}, []);

useEffect(() => {

fetchFile();

}, [fetchFile]);

greptile-apps · 2025-04-25T04:29:50Z

web/src/components/chat/TextView.tsx

  };

  const fetchFile = useCallback(async () => {
+    console.log("fetching file");


style: Remove debug console.log statements before merging to production

greptile-apps · 2025-04-25T04:30:21Z

web/src/components/filters/SourceSelector.tsx


  return (
-    <div>
+    <div className="b">


style: The 'b' class name appears to be a typo or undefined class. Consider using a more descriptive class name or removing if unnecessary.

Suggested change

<div className="b">

<div>

* update * improved my docs * nit * nit * k * push changes * update * looking good * k * fix preprocessing * try a fix * k * update * nit * k * quick nits * Cleanup / fixes * Fixes * Fix build * fix * fix quality checks --------- Co-authored-by: Weves <chrisweaver101@gmail.com>

pablonyx force-pushed the my_docs_cleanup branch from 285aba3 to ca89806 Compare April 13, 2025 23:36

vercel bot deployed to Preview April 13, 2025 23:40 View deployment

vercel bot deployed to Preview April 14, 2025 16:38 View deployment

vercel bot deployed to Preview April 15, 2025 17:56 View deployment

vercel bot deployed to Preview April 15, 2025 19:36 View deployment

pablonyx force-pushed the my_docs_cleanup branch from abe9e59 to 27b9456 Compare April 15, 2025 20:15

vercel bot deployed to Preview April 15, 2025 20:21 View deployment

vercel bot deployed to Preview April 16, 2025 16:40 View deployment

vercel bot deployed to Preview April 16, 2025 19:37 View deployment

vercel bot deployed to Preview April 17, 2025 00:35 View deployment

pablonyx force-pushed the my_docs_cleanup branch from 8bb0e81 to d3540aa Compare April 17, 2025 16:30

vercel bot deployed to Preview April 17, 2025 16:38 View deployment

vercel bot deployed to Preview April 17, 2025 20:01 View deployment

vercel bot deployed to Preview April 18, 2025 20:14 View deployment

vercel bot deployed to Preview April 18, 2025 20:56 View deployment

Weves force-pushed the my_docs_cleanup branch from a566bb1 to 6ba809d Compare April 24, 2025 18:26

vercel bot deployed to Preview April 24, 2025 18:31 View deployment

vercel bot had a problem deploying to Preview April 25, 2025 01:50 Failure

pablonyx added 11 commits April 24, 2025 21:12

update

5b74cff

improved my docs

4b1ba85

nit

1245d17

nit

af71072

k

4f253fb

push changes

f2c673e

update

307baa9

looking good

bb09261

k

336b174

fix preprocessing

ff6eafb

try a fix

c6548be

pablonyx and others added 8 commits April 24, 2025 21:12

k

ac076bb

update

7856cc2

nit

3d28211

k

b285acd

quick nits

82ea2f2

Cleanup / fixes

b1a2b01

Fixes

213d926

Fix build

c43d86d

Weves force-pushed the my_docs_cleanup branch from 48b278b to c43d86d Compare April 25, 2025 04:13

vercel bot deployed to Preview April 25, 2025 04:16 View deployment

Weves marked this pull request as ready for review April 25, 2025 04:20

Weves requested a review from a team as a code owner April 25, 2025 04:20

fix

f96f831

vercel bot deployed to Preview April 25, 2025 04:25 View deployment

fix quality checks

6b13e27

vercel bot deployed to Preview April 25, 2025 04:30 View deployment

greptile-apps bot reviewed Apr 25, 2025

View reviewed changes

Weves approved these changes Apr 25, 2025

View reviewed changes

Weves enabled auto-merge April 25, 2025 04:41

Weves added this pull request to the merge queue Apr 25, 2025

Merged via the queue into main with commit df67ca1 Apr 25, 2025
11 of 12 checks passed

Weves deleted the my_docs_cleanup branch April 25, 2025 06:13

This was referenced Apr 28, 2025

fix: LaTeX processing issue in #4570 #4571

Closed

LaTeX delimiters incorrectly escaped causing "$2" to appear instead of "$" in rendered math expressions #4570

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

My docs cleanup #4519

My docs cleanup #4519

Uh oh!

pablonyx commented Apr 13, 2025 •

edited by Weves

Loading

Uh oh!

vercel bot commented Apr 13, 2025 •

edited

Loading

Uh oh!

greptile-apps bot left a comment

Uh oh!

greptile-apps bot Apr 25, 2025

Uh oh!

greptile-apps bot Apr 25, 2025

Uh oh!

greptile-apps bot Apr 25, 2025

Uh oh!

greptile-apps bot Apr 25, 2025

Uh oh!

greptile-apps bot Apr 25, 2025

Uh oh!

greptile-apps bot Apr 25, 2025

Uh oh!

greptile-apps bot Apr 25, 2025

Uh oh!

greptile-apps bot Apr 25, 2025

Uh oh!

greptile-apps bot Apr 25, 2025

Uh oh!

greptile-apps bot Apr 25, 2025

Uh oh!

Uh oh!

Uh oh!

My docs cleanup #4519

My docs cleanup #4519

Uh oh!

Conversation

pablonyx commented Apr 13, 2025 • edited by Weves Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

How Has This Been Tested?

Backporting (check the box to trigger backport action)

Uh oh!

vercel bot commented Apr 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

Uh oh!

greptile-apps bot Apr 25, 2025

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Apr 25, 2025

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Apr 25, 2025

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Apr 25, 2025

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Apr 25, 2025

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Apr 25, 2025

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Apr 25, 2025

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Apr 25, 2025

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Apr 25, 2025

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Apr 25, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

pablonyx commented Apr 13, 2025 •

edited by Weves

Loading

vercel bot commented Apr 13, 2025 •

edited

Loading