Skip to content

epic(wren-ui): Adjust thread response answer#1512

Merged
andreashimin merged 7 commits intomainfrom
epic/adjust-answer
Apr 2, 2025
Merged

epic(wren-ui): Adjust thread response answer#1512
andreashimin merged 7 commits intomainfrom
epic/adjust-answer

Conversation

@andreashimin
Copy link
Copy Markdown
Contributor

@andreashimin andreashimin commented Apr 2, 2025

Related pull request

Summary by CodeRabbit

  • New Features

    • Introduced interactive adjustment capabilities for thread responses, allowing users to modify SQL and reasoning steps via new modals and dropdown options.
  • UI Enhancements

    • Upgraded content editing with a refreshed markdown editor and improved copy-to-clipboard experience.
    • Streamlined dropdown menus and action interfaces for smoother navigation and enhanced feedback.
  • Improved Reliability

    • Enhanced error messaging and telemetry provide more detailed, responsive feedback during adjustments.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 2, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

This pull request introduces comprehensive support for thread response adjustments across the stack. A new migration adds a JSONB column to store adjustment data. The GraphQL schema, types, and resolvers are extended with new input types and mutations (adjust, cancel, rerun) for thread responses. On the server side, adaptors, services, and a background task tracker are updated to manage feedback and adjustment tasks with improved error handling. Additionally, several UI components, hooks, and utilities are added or modified to support adjustment modals, dropdowns, and editing functionalities.

Changes

File(s) Change Summary
wren-ui/migrations/20250510000000_add_adjustment_to_thread_response.js Adds new migration to update the thread_response table with a nullable JSONB adjustment column and corresponding rollback functionality.
wren-ui/src/apollo/client/graphql/{__types__.ts, home.generated.ts, home.ts, schema.ts} Extends GraphQL schema with new types (e.g. AdjustThreadResponseInput, AdjustmentTask, ThreadResponseAdjustment), mutations (adjustThreadResponse, cancelAdjustmentTask, rerunAdjustmentTask), and updates the PreviewSQLDataInput.
wren-ui/src/apollo/server/adaptors/{wrenAIAdaptor.ts, wrenEngineAdaptor.ts} Adds methods for project deletion and feedback management in WrenAIAdaptor; enhances error handling in WrenEngineAdaptor using a custom error creation approach.
wren-ui/src/apollo/server/{backgrounds/adjustmentBackgroundTracker.ts, models/*, repositories/*, resolvers*, askingResolver.ts} Introduces AdjustmentBackgroundTaskTracker with task polling and management; updates resolvers and repositories to support new adjustment fields and handling of thread responses.
wren-ui/src/apollo/server/services/askingService.ts Adds new interfaces and methods for adjusting thread responses (both SQL and reasoning), canceling, rerunning adjustment tasks, and fetching task details.
wren-ui/src/common.ts Injects the askingTaskRepository as a new dependency during the initialization of the asking service.
wren-ui/src/components/{diagram/CustomDropdown.tsx, editor/*, modals/{AdjustReasoningStepsModal.tsx, AdjustSQLModal.tsx}, pages/home/*, promptThread/*} Introduces new and updated UI components and modals for adjustment actions; modifies dropdowns, editors, preparation status/steps, and answer rendering to integrate adjustment information.
wren-ui/src/hooks/{useAdjustAnswer.tsx, useDropdown.tsx, useMentions.tsx, useAskPrompt.tsx} Adds new hooks for managing adjustment tasks (useAdjustAnswer, useDropdown, useMentions), updates useAskPrompt to include adjustment task status, and removes the legacy useAnswerStepContent hook.
wren-ui/src/utils/{enum/*, error/*, svgs/*} Adds a new MORE_ACTION enum in dropdown utils, introduces the EditSVG component, updates error codes (e.g. WREN_ENGINE_ERROR), and adjusts export statements.

Sequence Diagram(s)

sequenceDiagram
    participant U as User
    participant UI as UI Component
    participant GQL as GraphQL Resolver
    participant SRV as AskingService
    participant ABT as AdjustmentBackgroundTaskTracker
    participant DB as Database

    U->>UI: Submit adjustment via modal
    UI->>GQL: adjustThreadResponse(responseId, data)
    GQL->>SRV: Process adjustment request
    SRV->>ABT: Create/Rerun adjustment task
    ABT->>DB: Update thread_response with adjustment data
    DB-->>ABT: Acknowledge update
    ABT-->>SRV: Return adjustment task info
    SRV-->>GQL: Return updated thread response
    GQL-->>UI: Send back adjusted response
Loading

Possibly related PRs

  • feat(wren-ui): Integrate chart_type & transform property #1117 – The changes in the main PR, which involve adding a new column adjustment to the thread_response table, are related to the retrieved PR as both involve modifications to the ThreadResponse structure, specifically in how adjustments and chart types are represented in the GraphQL schema.
  • fix(wren-ui): remove custom scale & add adjustment flag for adjust chart scenario #1062 – The changes in the main PR, which introduce a new column for adjustments in the thread_response table, are related to the retrieved PR, as both involve modifications to the ThreadResponse structure and its handling of adjustments, specifically through the addition of an adjustment property in various contexts.
  • feat(wren-ui): Reasoning enhancement & SQL pair in asking flow #1471 – The changes in the main PR, which involve adding a new column for adjustments in the thread_response table, are related to the retrieved PR as both involve enhancements to the handling of adjustments and SQL pairs in the context of thread responses. Specifically, the retrieved PR introduces new fields and types that accommodate SQL pairs, which are directly relevant to the adjustments being made in the main PR.

Suggested labels

module/ui

Suggested reviewers

  • onlyjackfrost
  • wwwy3y3
  • fredalai

Poem

I’m a bunny coder, hopping through code so neat,
Adjustments sprout like carrots—refreshingly sweet.
Migrations, modals, and mutations in a row,
With every commit, our project continues to grow.
In fields of code, I nuzzle and cheer,
For adjustments bring magic with every deploy near!
Hop on, dear coder, the rabbit leads the way!


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8052628 and 5509dc8.

📒 Files selected for processing (1)
  • wren-ui/src/apollo/server/resolvers/askingResolver.ts (6 hunks)
✨ Finishing Touches
  • 📝 Generate Docstrings

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai plan to trigger planning for file edits and PR creation.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@andreashimin andreashimin changed the title feat(wren-ui): Adjust thread response answer epic(wren-ui): Adjust thread response answer Apr 2, 2025
Copy link
Copy Markdown
Contributor

@fredalai fredalai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@andreashimin andreashimin merged commit 5126525 into main Apr 2, 2025
8 checks passed
@andreashimin andreashimin deleted the epic/adjust-answer branch April 2, 2025 05:59
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.

Actionable comments posted: 5

🧹 Nitpick comments (37)
wren-ui/src/apollo/server/utils/error.ts (1)

44-46: Consider adding corresponding error messages for the new error code.

The new WREN_ENGINE_ERROR code has been added to the GeneralErrorCodes enum, but it's missing corresponding entries in the errorMessages and shortMessages objects which are used throughout the application for error reporting.

Consider adding entries to both objects:

// Add to errorMessages object
+ [GeneralErrorCodes.WREN_ENGINE_ERROR]: 'An error occurred in the Wren engine',

// Add to shortMessages object
+ [GeneralErrorCodes.WREN_ENGINE_ERROR]: 'Wren engine error',
.github/workflows/ui-release-image-stable.yaml (1)

90-92: Added the 'latest' tag - LGTM but fix the trailing spaces.

Adding the 'latest' tag to Docker images is good practice for making the most recent stable version easily accessible. However, there are trailing spaces at the end of line 92 that should be removed.

-  TAGS=$(echo "${{ steps.meta.outputs.tags }} --tag latest" | awk '{printf "--tag %s ", $0}') 
+  TAGS=$(echo "${{ steps.meta.outputs.tags }} --tag latest" | awk '{printf "--tag %s ", $0}')
🧰 Tools
🪛 YAMLlint (1.35.1)

[error] 92-92: trailing spaces

(trailing-spaces)

docker/.env.example (1)

23-27: Version updates reflect stable release.

The version updates appropriately transition from release candidate (rc) to stable versions. The WREN_PRODUCT_VERSION change from 0.18.0-rc.1 to 0.18.0 indicates this is now a stable release, with corresponding updates to the AI service and UI versions.

Consider adding a brief comment about the significance of these version changes, especially noting the transition from RC to stable for WREN_PRODUCT_VERSION.

wren-ui/src/components/pages/home/preparation/PreparationStatus.tsx (2)

27-31: Consider renaming the stopAskingTask variable for clarity.

While the logic is correct, the variable name stopAskingTask might be misleading since it now handles both adjustment and asking tasks.

- const stopAskingTask = attachLoading(stopPreparedTask, setStopLoading);
+ const stopPreparedTask = attachLoading(stopPreparedTask, setStopLoading);

36-40: Consider renaming the reRunAskingTask variable for clarity.

Similar to the previous comment, the variable name reRunAskingTask could be more accurately named to reflect that it handles both types of tasks.

- const reRunAskingTask = attachLoading(reRunPreparedTask, setReRunLoading);
+ const reRunPreparedTask = attachLoading(reRunPreparedTask, setReRunLoading);
wren-ui/src/components/pages/home/preparation/index.tsx (1)

33-45: Improve type safety in the preparedTask memoization.

The implementation effectively merges task types, but the type assertion at the end could be improved.

Consider using a more type-safe approach instead of using a type assertion:

- return {
-   candidates: [],
-   invalidSql: '',
-   retrievedTables: payload?.retrievedTables || [],
-   sqlGenerationReasoning: payload?.sqlGenerationReasoning || '',
-   isAdjustment: !!adjustmentTask,
-   ...(askingTask || {}),
-   ...(adjustmentTask || {}),
- } as PreparedTask;
+ // Create base object with required fields
+ const baseTask: Partial<PreparedTask> = {
+   candidates: [],
+   invalidSql: '',
+   retrievedTables: payload?.retrievedTables || [],
+   sqlGenerationReasoning: payload?.sqlGenerationReasoning || '',
+   isAdjustment: !!adjustmentTask
+ };
+ 
+ // Merge with asking/adjustment task data
+ return {
+   ...baseTask,
+   ...(askingTask || {}),
+   ...(adjustmentTask || {})
+ } as PreparedTask;
wren-ui/src/components/modals/AdjustReasoningStepsModal.tsx (3)

46-66: Add proper typing for model options.

The implementation for model options looks good, but remember to type the models appropriately.

Consider adding proper types:

- const modelOptions = useMemo(() => {
+ const modelOptions = useMemo<Array<{label: string, value: string}>>(() => {
    return listModelsResult.data?.listModels.map((model) => ({
      label: model.displayName,
      value: model.referenceName,
    }));
  }, [listModelsResult.data?.listModels]);

52-66: Remove debugging console.log statement.

There's a console.log statement that should be removed before production.

  if (defaultValue?.retrievedTables.includes(model.referenceName)) {
-    console.log(model.referenceName);
    result.push({ label: model.displayName, value: model.referenceName });
  }

68-92: Add proper typing for tagRender function props.

The tagRender function has untyped props which could lead to type errors.

- const tagRender = (props) => {
+ const tagRender = (props: { value: string; closable: boolean; onClose: () => void }) => {
    const { value, closable, onClose } = props;
    const model = modelNameMap[value];
    // ...
.github/workflows/create-rc-release.yaml (3)

33-40: Improve version extraction reliability.

The current method of extracting the release version relies on splitting by spaces and taking the last item, which could be error-prone if issue titles don't follow a consistent format.

 - name: Parse release version from PR title
   id: parse_release_version
   env:
     GITHUB_ISSUE_TITLE: ${{ github.event.issue.title }}
   run: |
-    release_version=$(echo "$GITHUB_ISSUE_TITLE" | sed 's/ /\n/g' | tail -n 1)
+    # Extract version using regex pattern for semantic versioning
+    release_version=$(echo "$GITHUB_ISSUE_TITLE" | grep -o 'v\?[0-9]\+\.[0-9]\+\.[0-9]\+\(-[a-zA-Z0-9]\+\)\?' || echo "")
+    if [ -z "$release_version" ]; then
+      echo "Error: Could not extract valid release version from title: $GITHUB_ISSUE_TITLE"
+      exit 1
+    fi
     echo "Release version: $release_version"
     echo "release_version=$release_version" >> $GITHUB_OUTPUT

27-31: Use GitHub API action instead of direct curl command.

Using direct curl commands with tokens in the command line can pose security risks. Consider using the GitHub API action instead.

 - name: Add rocket emoji to comment
-  run: |
-    curl -X POST -H "Authorization: token ${{ secrets.GITHUB_TOKEN }}" \
-    -d '{"body": "🚀 Starting the release process!"}' \
-    "https://api.github.com/repos/${{ github.repository }}/issues/${{ github.event.issue.number }}/comments"
+  uses: peter-evans/create-or-update-comment@v3
+  with:
+    issue-number: ${{ github.event.issue.number }}
+    body: "🚀 Starting the release process!"
+    token: ${{ secrets.GITHUB_TOKEN }}

76-81: Use GitHub API action for commenting with release link.

Similar to the previous comment, consider using a dedicated GitHub action for creating comments.

 - name: Comment with release link
-  run: |
-    release_url="https://github.com/${{ github.repository }}/releases/tag/${{ steps.parse_release_version.outputs.release_version }}"
-    curl -X POST -H "Authorization: token ${{ secrets.GITHUB_TOKEN }}" \
-    -d "{\"body\": \"🚀 A new release has been created! [View Release](${release_url})\"}" \
-    "https://api.github.com/repos/${{ github.repository }}/issues/${{ github.event.issue.number }}/comments"
+  uses: peter-evans/create-or-update-comment@v3
+  with:
+    issue-number: ${{ github.event.issue.number }}
+    body: "🚀 A new release has been created! [View Release](https://github.com/${{ github.repository }}/releases/tag/${{ steps.parse_release_version.outputs.release_version }})"
+    token: ${{ secrets.GITHUB_TOKEN }}
wren-ui/src/components/pages/home/promptThread/ViewSQLTabContent.tsx (2)

60-64: Update useEffect dependency array.

The useEffect dependency array doesn't include autoTriggerPreviewDataButton and onInitPreviewDone, which could lead to stale closures.

 useEffect(() => {
   if (isLastThreadResponse) {
     autoTriggerPreviewDataButton();
   }
-}, [isLastThreadResponse]);
+}, [isLastThreadResponse, autoTriggerPreviewDataButton, onInitPreviewDone]);

44-46: Enhance error handling for preview data mutation.

Current error handling only logs to console. Consider adding user-visible error feedback.

 const [previewData, previewDataResult] = usePreviewDataMutation({
-  onError: (error) => console.error(error),
+  onError: (error) => {
+    console.error(error);
+    // Consider adding a notification or inline error message
+    // Example: notification.error({ message: 'Failed to preview data', description: error.message });
+  },
 });
wren-ui/src/components/modals/AdjustSQLModal.tsx (4)

62-66: Add type definition for error parameter.

The handleError function has an untyped error parameter, which could lead to issues if the error structure changes.

-const handleError = (error) => {
+const handleError = (error: unknown) => {
   const graphQLError = parseGraphQLError(error);
   setError({ ...graphQLError, shortMessage: 'Invalid SQL syntax' });
   console.error(graphQLError);
 };

68-88: Prevent state updates on unmounted components.

There's a potential memory leak in the async functions, as they don't check if the component is still mounted before updating state.

+const [isMounted, setIsMounted] = useState(true);
+
+useEffect(() => {
+  return () => {
+    setIsMounted(false);
+  };
+}, []);
+
 const onPreviewData = async () => {
+  if (!isMounted) return;
   setError(null);
   setPreviewing(true);
   try {
     await onValidateSQL();
+    if (!isMounted) return;
     setShowPreview(true);
     await previewSqlMutation({
       variables: {
         data: {
           sql: sqlValue,
           limit: 50,
         },
       },
     });
   } catch (error) {
+    if (!isMounted) return;
     setShowPreview(false);
     handleError(error);
   } finally {
+    if (!isMounted) return;
     setPreviewing(false);
   }
 };

120-129: Update deprecated 'visible' prop to 'open'.

The modal uses the visible prop which is deprecated in Ant Design v4 and should be replaced with open.

 <Modal
   title="Adjust SQL"
   centered
   closable
   confirmLoading={confirmLoading}
   destroyOnClose
   maskClosable={false}
   onCancel={onClose}
-  visible={visible}
+  open={visible}
   width={640}

90-114: Add form dirty state check for unsaved changes.

Consider adding a confirmation dialog when closing the modal with unsaved changes.

+const [formDirty, setFormDirty] = useState(false);
+
+useEffect(() => {
+  if (sqlValue !== defaultValue?.sql) {
+    setFormDirty(true);
+  }
+}, [sqlValue, defaultValue?.sql]);
+
+const handleCancel = () => {
+  if (formDirty && sqlValue) {
+    Modal.confirm({
+      title: 'Discard changes?',
+      content: 'You have unsaved changes. Are you sure you want to discard them?',
+      onOk: () => {
+        onClose();
+      },
+    });
+  } else {
+    onClose();
+  }
+};

// Then replace onCancel={onClose} with onCancel={handleCancel}
wren-ui/src/components/editor/MarkdownEditor.tsx (4)

32-34: Fix typo in CSS property name.

There's a typo in the CSS property: "borer-color" should be "border-color".

 &.adm-markdown-editor-focused.adm-markdown-editor-error {
-  borer-color: var(--red-4) !important;
+  border-color: var(--red-4) !important;
   box-shadow: 0 0 0 2px rgba(255, 77, 79, 0.2);
 }

131-164: Enhance Markdown editing with support for multi-line code blocks.

The keyboard event handling for backticks doesn't support multi-line code blocks, which is a common Markdown feature.

 if (e.key === '`') {
   const textarea = e.currentTarget;
   const start = textarea.selectionStart;
   const end = textarea.selectionEnd;

   if (start !== end) {
     e.preventDefault();
-    const selection = `\`${value?.slice(start, end)}\``;
+    const selectedText = value?.slice(start, end) || '';
+    let selection;
+    
+    // Check if selection spans multiple lines
+    if (selectedText.includes('\n')) {
+      selection = `\`\`\`\n${selectedText}\n\`\`\``;
+    } else {
+      selection = `\`${selectedText}\``;
+    }
+    
     const newValue = value?.slice(0, start) + selection + value?.slice(end);
     // update the value and move the cursor
     onChange?.(newValue || '');
     nextTick().then(() => {
       textarea.selectionStart = textarea.selectionEnd =
         start + selection.length;
     });
   }
 }

165-198: Extract dropdown menu scrolling logic into a separate function.

The dropdown menu scrolling logic is complex and might benefit from being extracted into a separate function for clarity.

+const handleDropdownScroll = (dropdownMenu: Element, direction: 'up' | 'down') => {
+  // delay to make sure the menu active item is rendered
+  nextTick().then(() => {
+    const activeItem = dropdownMenu.querySelector(
+      '.ant-mentions-dropdown-menu-item-active',
+    ) as HTMLLIElement;
+    if (activeItem) {
+      const menuRect = dropdownMenu.getBoundingClientRect();
+      const activeRect = activeItem.getBoundingClientRect();
+      // check if active item is outside viewport
+      if (direction === 'down' && activeRect.bottom > menuRect.bottom) {
+        // scroll down
+        dropdownMenu.scrollTo({
+          top:
+            dropdownMenu.scrollTop +
+            (activeRect.bottom - menuRect.bottom),
+          behavior: 'smooth',
+        });
+      } else if (direction === 'up' && activeRect.top < menuRect.top) {
+        // scroll up
+        dropdownMenu.scrollTo({
+          top: dropdownMenu.scrollTop - (menuRect.top - activeRect.top),
+          behavior: 'smooth',
+        });
+      }
+    }
+  });
+};

 if (e.key === 'ArrowDown' || e.key === 'ArrowUp') {
   // check if the mention dropdown menu exist
   const dropdownMenu = $wrapper.current?.querySelector(
     '.ant-mentions-dropdown-menu',
   );
   if (dropdownMenu) {
-    // delay to make sure the menu active item is rendered
-    nextTick().then(() => {
-      const activeItem = dropdownMenu.querySelector(
-        '.ant-mentions-dropdown-menu-item-active',
-      ) as HTMLLIElement;
-      if (activeItem) {
-        const menuRect = dropdownMenu.getBoundingClientRect();
-        const activeRect = activeItem.getBoundingClientRect();
-        // check if active item is outside viewport
-        if (activeRect.bottom > menuRect.bottom) {
-          // scroll down
-          dropdownMenu.scrollTo({
-            top:
-              dropdownMenu.scrollTop +
-              (activeRect.bottom - menuRect.bottom),
-            behavior: 'smooth',
-          });
-        } else if (activeRect.top < menuRect.top) {
-          // scroll up
-          dropdownMenu.scrollTo({
-            top: dropdownMenu.scrollTop - (menuRect.top - activeRect.top),
-            behavior: 'smooth',
-          });
-        }
-      }
-    });
+    handleDropdownScroll(dropdownMenu, e.key === 'ArrowDown' ? 'down' : 'up');
   }
 }

55-61: Consider adding more Markdown editing features.

While the current implementation provides basic Markdown editing with mentions support, it could be enhanced with buttons for common Markdown elements like headers, lists, or links to improve usability.

Consider adding a toolbar with buttons for common Markdown formatting options:

+interface ToolbarProps {
+  onInsert: (text: string) => void;
+}
+
+const MarkdownToolbar = ({ onInsert }: ToolbarProps) => {
+  const insertFormat = (prefix: string, suffix: string = '') => {
+    const textarea = $textarea.current?.textarea;
+    if (!textarea) return;
+    
+    const start = textarea.selectionStart;
+    const end = textarea.selectionEnd;
+    const selection = (value || '').slice(start, end);
+    const newValue = (value || '').slice(0, start) + prefix + selection + suffix + (value || '').slice(end);
+    onChange?.(newValue);
+    
+    nextTick().then(() => {
+      textarea.focus();
+      if (selection) {
+        textarea.selectionStart = start + prefix.length;
+        textarea.selectionEnd = start + prefix.length + selection.length;
+      } else {
+        textarea.selectionStart = textarea.selectionEnd = start + prefix.length;
+      }
+    });
+  };
+  
+  return (
+    <Space className="mb-2">
+      <Button size="small" onClick={() => insertFormat('# ')}>H1</Button>
+      <Button size="small" onClick={() => insertFormat('## ')}>H2</Button>
+      <Button size="small" onClick={() => insertFormat('- ')}>List</Button>
+      <Button size="small" onClick={() => insertFormat('1. ')}>Numbered</Button>
+      <Button size="small" onClick={() => insertFormat('[', '](url)')}>Link</Button>
+      <Button size="small" onClick={() => insertFormat('**', '**')}>Bold</Button>
+      <Button size="small" onClick={() => insertFormat('*', '*')}>Italic</Button>
+    </Space>
+  );
+};

// Add the toolbar above the editor
// Inside the OverflowContainer, before the conditional rendering
wren-ui/src/components/pages/home/promptThread/TextBasedAnswer.tsx (1)

165-165: Minor styling update.
Switching to className="m-6" for the Alert is purely presentational; no functional issues identified.

wren-ui/src/apollo/server/repositories/threadResponseRepository.ts (1)

58-63: Consider refactoring the intersection payload to a discriminated union.
It is currently using an intersection of two payload types. A union type with a well-defined discriminator may better separate "reasoning" vs. "apply SQL" cases.

wren-ui/src/apollo/client/graphql/__types__.ts (4)

26-30: Add clarifying doc comments for better maintainability
Having JSDoc-like descriptions for each optional field (sql, sqlGenerationReasoning, and tables) will improve readability and future maintainability.


640-642: Cancellation argument
Consider also capturing an optional reason or context for cancellation to aid in debugging or auditing tasks.


1023-1025: Check non-existent tasks
If taskId does not exist, ensure proper error handling or a null return is implemented in the resolver.


1256-1265: Enum-based approach for clarity
ThreadResponseAdjustment and ThreadResponseAdjustmentType nicely clarify whether you're applying SQL or additional reasoning. Consider adding doc comments describing each.

wren-ui/src/apollo/server/services/askingService.ts (2)

417-418: New private fields
askingTaskRepository and adjustmentBackgroundTracker look well-defined. Check for consistent field ordering if there's a style guideline.


483-488: Initializing the background tracker
The new AdjustmentBackgroundTaskTracker is passed the correct dependencies. Check resource usage to ensure it won't conflict with other background tasks.

wren-ui/src/apollo/server/backgrounds/adjustmentBackgroundTracker.ts (7)

69-82: Class definition
Declaring internal collections (trackedTasks, trackedTasksById) for state management is straightforward but ensure memory usage remains bounded if tasks accumulate quickly.


83-105: Initialization logic
Starting polling automatically is convenient. Allowing an opt-out or manual start could be helpful for large-scale environments.


107-174: Create workflow
The method calls the AI service, stores the newly created thread response, and begins tracking the task. Looks good. Make sure error handling for partially created tasks is robust.


281-289: Cancellation logic
cancelAskFeedback is invoked, paired with telemetry logging. Consider how to handle partial cancellations if the AI service is mid-processing.


297-301: Automated intervals
Periodic polling operates at a 1-second interval by default. This might be too frequent for high-volume usage and can be made configurable.


421-431: Thread response finalization
Updating the thread response SQL is a key final step. Consider logging old SQL vs new SQL for auditability or rollback.


491-501: Result comparison
Comparing statuses is a minimal approach. Consider comparing error states or partial results if needed for advanced use cases.

🛑 Comments failed to post (5)
.github/workflows/create-rc-release.yaml (1)

22-25: ⚠️ Potential issue

Update Go version and action reference.

The Go version "1.23.0" appears to be invalid or future-dated. As of April 2025, the latest stable Go version is likely lower. Additionally, the static analysis indicates that the actions/setup-go@v4 action may be too old for the GitHub Actions runner.

 - name: Set up Go
-  uses: actions/setup-go@v4
+  uses: actions/setup-go@v5
   with:
-    go-version: "1.23.0"
+    go-version: "1.22.x"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

      - name: Set up Go
        uses: actions/setup-go@v5
        with:
          go-version: "1.22.x"
🧰 Tools
🪛 actionlint (1.7.4)

23-23: the runner of "actions/setup-go@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue

(action)

wren-ui/src/components/pages/home/promptThread/ViewSQLTabContent.tsx (2)

52-57: ⚠️ Potential issue

Add null check for callback function.

The function calls onInitPreviewDone() without checking if it exists, which could cause errors if onInitPreviewDone is not provided.

 const autoTriggerPreviewDataButton = async () => {
   await nextTick();
   await onPreviewData();
   await nextTick();
-  onInitPreviewDone();
+  onInitPreviewDone?.();
 };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

  const autoTriggerPreviewDataButton = async () => {
    await nextTick();
    await onPreviewData();
    await nextTick();
    onInitPreviewDone?.();
  };

48-50: ⚠️ Potential issue

Fix reference to undefined variable id.

The id variable is being referenced before it's defined (it's defined on line 66). This could lead to undefined behavior if the component rerenders before id is available.

 const onPreviewData = async () => {
+  const { id } = threadResponse;
   await previewData({ variables: { where: { responseId: id } } });
 };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

  const onPreviewData = async () => {
    const { id } = threadResponse;
    await previewData({ variables: { where: { responseId: id } } });
  };
wren-ui/src/components/editor/MarkdownEditor.tsx (1)

112-129: ⚠️ Potential issue

Add null check for value in select function.

The select function assumes that value exists when calculating mentionStart, which could lead to errors if value is undefined.

 const select = (option: Mention) => {
   const textarea = $textarea.current?.textarea;
   if (!textarea) return;

   // go to the start of the mention
   const mentionStart = (
-    value?.slice(0, textarea.selectionStart) || ''
+    (value || '').slice(0, textarea.selectionStart)
   ).lastIndexOf(MENTION_PREFIX);
   const start = mentionStart >= 0 ? mentionStart : textarea.selectionStart;
   const end = textarea.selectionEnd;
-  const newValue = value?.slice(0, start) + option.value + value?.slice(end);
+  const newValue = (value || '').slice(0, start) + option.value + (value || '').slice(end);
   // update the value and move the cursor
   onChange?.(newValue || '');
   nextTick().then(() => {
     textarea.selectionStart = textarea.selectionEnd =
       start + option.value.length;
   });
 };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

  const select = (option: Mention) => {
    const textarea = $textarea.current?.textarea;
    if (!textarea) return;

    // go to the start of the mention
    const mentionStart = (
      (value || '').slice(0, textarea.selectionStart)
    ).lastIndexOf(MENTION_PREFIX);
    const start = mentionStart >= 0 ? mentionStart : textarea.selectionStart;
    const end = textarea.selectionEnd;
    const newValue = (value || '').slice(0, start) + option.value + (value || '').slice(end);
    // update the value and move the cursor
    onChange?.(newValue || '');
    nextTick().then(() => {
      textarea.selectionStart = textarea.selectionEnd =
        start + option.value.length;
    });
  };
wren-ui/src/apollo/server/services/askingService.ts (1)

1087-1112: 🛠️ Refactor suggestion

Implementing adjustThreadResponseAnswer
This method delegates the actual adjustment logic to adjustmentBackgroundTracker.createAdjustmentTask. Consider ensuring the original thread response is in a valid state before adjusting again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants