Skip to content

Conversation

sid597
Copy link
Collaborator

@sid597 sid597 commented Mar 7, 2025

  • gemini
  • anthropic
  • openai

Summary by CodeRabbit

  • New Features

    • Added new chat endpoints integrating multiple conversational AI services, enhancing the platform's chat capabilities.
    • Introduced utilities for improved request processing and secure cross-origin interactions.
    • Standardized configurations for language model interactions to ensure consistent and reliable behavior.
  • Chores

    • Updated website runtime settings for greater reliability, including stricter React mode.
    • Expanded build environment configurations to support additional operational keys.

Copy link

vercel bot commented Mar 7, 2025

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

Name Status Preview Comments Updated (UTC)
discourse-graph ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 8, 2025 0:43am

Copy link
Contributor

coderabbitai bot commented Mar 7, 2025

Important

Review skipped

Auto reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

📝 Walkthrough

Walkthrough

The changes update the Next.js configuration, add new API endpoints for Anthropic, Gemini, and OpenAI chat interactions, and enhance the build environment setup. New TypeScript files introduce API routes with exported POST and OPTIONS handlers, along with shared utility functions for request handling and CORS. Additionally, several new types and provider configurations for LLM interactions have been added. The modifications also update build configurations in turbo.json to pass through additional environment variables.

Changes

File(s) Change Summary
apps/website/next.config.ts Removed type annotation for nextConfig; set reactStrictMode to true and added serverRuntimeConfig with maxDuration: 300.
apps/website/app/api/llm/anthropic/chat/route.ts,
apps/website/app/api/llm/gemini/chat/route.ts,
apps/website/app/api/llm/openai/chat/route.ts
Introduced new API routes for LLM chat requests with exported POST and OPTIONS methods. The OpenAI route additionally exports runtime, preferredRegion, and maxDuration.
turbo.json Updated the build task’s passThroughEnv list to include additional environment variables: "OPENAI_API_KEY", "ANTHROPIC_API_KEY", "GEMINI_API_KEY", and "BLOB_READ_WRITE_TOKEN".
apps/website/app/types/llm.ts Added new types (Message, Settings, RequestBody, LLMProviderConfig) and constants (CONTENT_TYPE_JSON, CONTENT_TYPE_TEXT) for LLM communication.
apps/website/app/utils/llm/cors.ts Added a CORS utility that defines allowed origins and applies CORS headers based on request origin and method.
apps/website/app/utils/llm/handler.ts Introduced asynchronous functions handleLLMRequest and handleOptionsRequest to process API requests and preflight checks with proper error handling.
apps/website/app/utils/llm/providers.ts Added provider configurations for OpenAI, Gemini, and Anthropic as LLMProviderConfig objects, each outlining API URL, headers, request formatting, and response extraction.

Sequence Diagram(s)

sequenceDiagram
    participant C as Client
    participant AR as API Route
    participant H as Handler
    participant LP as LLM Provider

    C->>AR: HTTP Request (POST/OPTIONS)
    alt POST Request
        AR->>H: handleLLMRequest(request, config)
        H->>LP: Send formatted API request
        LP-->>H: API response
        H-->>AR: Formulated response
    else OPTIONS Request
        AR->>H: handleOptionsRequest(request)
        H-->>AR: 204 Response with CORS headers
    end
    AR-->>C: Return response with CORS headers
Loading

Poem

I hopped through lines of code so bright,
New routes and types gave my heart delight.
Configs updated with precision and cheer,
API calls now shine crystal clear.
From one bunny's trail to another's embrace,
Every change makes this garden a happier place!


🪧 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.

@mdroidian
Copy link
Contributor

As discussed, let's use the apps/website api endpoint for this

@mdroidian mdroidian marked this pull request as draft March 13, 2025 20:48
@sid597
Copy link
Collaborator Author

sid597 commented Mar 30, 2025

@coderabbitai review

Copy link
Contributor

coderabbitai bot commented Mar 30, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
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.

Actionable comments posted: 1

🧹 Nitpick comments (6)
apps/website/app/api/llm-endpoints/chat-anthropic/route.ts (1)

82-85: Safeguard against missing usage data.
The code assumes responseData.usage always exists. If usage is absent from the API response, accessing usage.input_tokens could result in a runtime error. Consider adding a check to gracefully handle scenarios where the usage object is missing.

- const usage: AnthropicUsage = responseData.usage;
- const inputTokens = usage.input_tokens;
- const outputTokens = usage.output_tokens;
+ const usage: AnthropicUsage | undefined = responseData.usage;
+ const inputTokens = usage?.input_tokens ?? 0;
+ const outputTokens = usage?.output_tokens ?? 0;
apps/website/app/api/llm-endpoints/chat-openai/route.ts (1)

46-53: Validate optional fields.
When selectively adding temperature and max_tokens to the payload, ensure they align with OpenAI’s supported parameter ranges. Additionally, consider validating them to avoid unexpected 400 errors or unbounded generation.

apps/website/next.config.ts (4)

3-3: Consider adding explicit type annotation for better type safety.

The explicit NextConfig type annotation has been removed from the declaration. While TypeScript can infer types, explicit typing provides better documentation and type safety.

-const nextConfig = {
+const nextConfig: NextConfig = {

10-12: Consider if the long maxDuration is necessary for all endpoints.

Setting maxDuration to 300 seconds (5 minutes) is quite long for serverless functions and may impact costs. Consider if this duration is necessary for all endpoints or if it could be reduced for certain operations.

You might want to:

  1. Set different durations for different API routes based on their expected execution time
  2. Implement timeout handling to gracefully handle long-running operations
  3. Consider optimizing the more time-consuming operations

13-15: Ensure environment variables are properly documented.

The configuration sets the NEXT_PUBLIC_APP_ENV environment variable with a default value of "development". Make sure this is documented in your project's README or environment setup guide.

  env: {
    NEXT_PUBLIC_APP_ENV: process.env.NEXT_PUBLIC_APP_ENV || "development",
+    // Add any other environment variables needed for the LLM endpoints here
  },

3-16: Consider adding configuration comments for better documentation.

The Next.js configuration contains several important settings but lacks comments explaining their purpose. Adding descriptive comments would make the configuration more maintainable.

const nextConfig = {
+  // Enable additional checks during development
  reactStrictMode: true,
  experimental: {
    serverActions: {
+      // Restrict server actions to specific origins for security
      allowedOrigins: ["localhost:3000", "discoursegraphs.com"],
    },
  },
+  // Configure server runtime for long-running LLM API requests
  serverRuntimeConfig: {
    maxDuration: 300,
  },
+  // Public environment variables accessible by browser code
  env: {
    NEXT_PUBLIC_APP_ENV: process.env.NEXT_PUBLIC_APP_ENV || "development",
  },
};
📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 0dd45ef and 331cd10.

📒 Files selected for processing (4)
  • apps/website/app/api/llm-endpoints/chat-anthropic/route.ts (1 hunks)
  • apps/website/app/api/llm-endpoints/chat-gemini/route.ts (1 hunks)
  • apps/website/app/api/llm-endpoints/chat-openai/route.ts (1 hunks)
  • apps/website/next.config.ts (1 hunks)
🔇 Additional comments (8)
apps/website/app/api/llm-endpoints/chat-anthropic/route.ts (2)

14-18: Confirm the role of the passphrase.
The passphrase field in RequestBody is neither validated nor used within the function. If you intend to restrict access or perform any secure operation, consider implementing logic to handle this parameter. Otherwise, remove it to avoid confusion.


120-125: Good standardized error response.
The createErrorResponse function provides a consistent structure for error handling. This approach is clear and reusable.

apps/website/app/api/llm-endpoints/chat-gemini/route.ts (2)

21-24: Verify passphrase usage.
A passphrase field is present but never validated or used. If this is intended to gate access, add the necessary authorization checks. Otherwise, consider removing it to reduce complexity.


122-127: Error response helper is properly structured.
The centralized error-handling strategy is straightforward and ensures a uniform error format.

apps/website/app/api/llm-endpoints/chat-openai/route.ts (2)

15-18: Confirm the intended use of passphrase.
The passphrase property is present but never used. Confirm whether you plan to enforce a secure check or remove this property if it’s unnecessary.


98-103: Consistent error response pattern.
The error-handling approach matches the style used in other endpoints, promoting clarity and standardization.

apps/website/next.config.ts (2)

4-4: Good addition of strict mode.

Enabling React strict mode helps identify potential problems in the application during development by performing additional checks and warnings.


5-9:

❓ Verification inconclusive

Verify that the allowed origins list is complete for your environments.

The allowedOrigins configuration restricts which domains can invoke server actions. Ensure this list includes all necessary domains for your production, staging, and development environments.


🏁 Script executed:

#!/bin/bash
# Check if there are any other domains or URLs used in the codebase that might need to be added to allowedOrigins
echo "Checking for potential domains that might need to be in allowedOrigins..."
rg -i "https?://[^/\"']+" --no-filename | grep -v "localhost:3000\|discoursegraphs.com" | sort | uniq

Length of output: 90371


Action Required: Verify Allowed Origins List Completeness

The current configuration in apps/website/next.config.ts specifies only "localhost:3000" and "discoursegraphs.com". A scan of the codebase has surfaced many external URLs (for example, references such as http://localhost:3003, https://samepage.ngrok.io, and various third‐party domains). Please double-check and confirm that the list in allowedOrigins includes every domain that may legitimately trigger server actions in your production, staging, and development setups. At the same time, ensure that domains referenced in static assets (like embedded YouTube URLs or external documentation links) are not mistakenly added unless they are expected to invoke server actions.

@sid597
Copy link
Collaborator Author

sid597 commented Apr 3, 2025

@coderabbitai review

@sid597 sid597 marked this pull request as ready for review April 3, 2025 03:01
@sid597 sid597 requested a review from mdroidian April 3, 2025 03:01
Copy link
Contributor

coderabbitai bot commented Apr 3, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
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.

Actionable comments posted: 4

🧹 Nitpick comments (8)
apps/website/next.config.ts (1)

3-3: Consider adding back the type annotation for clarity.

The AI summary indicates that the type annotation : NextConfig was removed from the constant declaration. While TypeScript can infer the type, explicit typing can improve code readability and maintainability.

- const nextConfig = {
+ const nextConfig: NextConfig = {
apps/website/lib/llm/types.ts (2)

3-6: Consider using a union type for role to improve type safety.

The role property is defined as a simple string, but LLM providers typically expect specific role values (like "user", "assistant", "system"). Using a union type would provide better type safety and code completion.

export type Message = {
-  role: string;
+  role: "user" | "assistant" | "system" | string;
  content: string;
};

18-22: Rename documents field to messages for clarity.

The field documents contains an array of Message objects, so naming it messages would be more intuitive and consistent with the actual data type it holds.

export type RequestBody = {
-  documents: Message[];
+  messages: Message[];
  passphrase?: string;
  settings: Settings;
};
apps/website/lib/llm/providers.ts (2)

29-32: Improve role mapping logic for Gemini.

The current role mapping is binary (either "user" or "model"), which might not handle other roles like "system" correctly. Consider adding explicit handling for system messages or other special roles.

  formatRequestBody: (messages: Message[], settings: Settings) => ({
    contents: messages.map((msg) => ({
-      role: msg.role === "user" ? "user" : "model",
+      role: msg.role === "user" ? "user" : 
+           msg.role === "system" ? "system" : "model",
      parts: [{ text: msg.content }],
    })),

48-51: Consider using a variable for the Anthropic API version.

The Anthropic API version is hardcoded, which might lead to issues if the API is updated. Using a constant or environment variable would make it easier to update in the future.

+const ANTHROPIC_API_VERSION = "2023-06-01";

// Later in the config
  apiHeaders: (apiKey: string) => ({
    "Content-Type": "application/json",
    "x-api-key": apiKey,
-    "anthropic-version": "2023-06-01",
+    "anthropic-version": ANTHROPIC_API_VERSION,
  }),
apps/website/lib/llm/handler.ts (3)

26-40: Use appropriate HTTP status code for missing API keys.

Using HTTP status code 500 (Internal Server Error) for a missing API key isn't appropriate as it's a configuration issue, not a server error. Consider using 503 (Service Unavailable) instead.

  if (!apiKey) {
    console.error(`${config.apiKeyEnvVar} environment variable is not set`);
    return cors(
      request,
      new Response(
        JSON.stringify({
          error: `API key not configured. Please set the ${config.apiKeyEnvVar} environment variable in your Vercel project settings.`,
        }),
        {
-          status: 500,
+          status: 503,
          headers: { "Content-Type": CONTENT_TYPE_JSON },
        },
      ),
    );
  }

98-105: Limit error details in production responses.

The current error handling exposes full error messages to clients, which could potentially leak sensitive information. Consider limiting error details in production environments while still logging the full error for debugging.

  catch (error) {
    console.error("Error processing request:", error);
+   const isProduction = process.env.NODE_ENV === 'production';
    return cors(
      request,
      new Response(
        JSON.stringify({
-         error: `Internal Server Error: ${error instanceof Error ? error.message : "Unknown error"}`,
+         error: isProduction ? "Internal Server Error" : `Internal Server Error: ${error instanceof Error ? error.message : "Unknown error"}`,
        }),

21-22: Update RequestBody field name in destructuring to match earlier recommendation.

If you implement the earlier suggestion to rename the documents field to messages in the RequestBody type, you'll need to update this destructuring assignment as well.

    const requestData: RequestBody = await request.json();
-    const { documents: messages, settings } = requestData;
+    const { messages, settings } = requestData;
📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between a5f7276 and 2b99736.

📒 Files selected for processing (7)
  • apps/website/app/api/llm/anthropic/chat/route.ts (1 hunks)
  • apps/website/app/api/llm/gemini/chat/route.ts (1 hunks)
  • apps/website/app/api/llm/openai/chat/route.ts (1 hunks)
  • apps/website/lib/llm/handler.ts (1 hunks)
  • apps/website/lib/llm/providers.ts (1 hunks)
  • apps/website/lib/llm/types.ts (1 hunks)
  • apps/website/next.config.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • apps/website/app/api/llm/openai/chat/route.ts
  • apps/website/app/api/llm/anthropic/chat/route.ts
  • apps/website/app/api/llm/gemini/chat/route.ts
🧰 Additional context used
🧬 Code Definitions (2)
apps/website/lib/llm/providers.ts (1)
apps/website/lib/llm/types.ts (3)
  • LLMProviderConfig (27-34)
  • Message (3-6)
  • Settings (8-16)
apps/website/lib/llm/handler.ts (1)
apps/website/lib/llm/types.ts (4)
  • LLMProviderConfig (27-34)
  • RequestBody (18-22)
  • CONTENT_TYPE_JSON (24-24)
  • CONTENT_TYPE_TEXT (25-25)
🪛 Biome (1.9.4)
apps/website/lib/llm/handler.ts

[error] 63-63: eval() exposes to security risks and performance issues.

See the MDN web docs for more details.
Refactor the code so that it doesn't need to call eval().

(lint/security/noGlobalEval)

🔇 Additional comments (1)
apps/website/next.config.ts (1)

3-11: Solid configuration for Vercel serverless functions!

The changes to the Next.js configuration appropriately support the migration of LLM API endpoints to Vercel serverless:

  1. reactStrictMode: true enables additional development-time checks and warnings that help catch potential issues early.

  2. serverRuntimeConfig.maxDuration: 300 sets a 5-minute timeout for serverless functions, which is appropriate for LLM API calls that might take longer to process.

  3. The environment variable configuration provides a good default for NEXT_PUBLIC_APP_ENV.

Copy link
Contributor

@mdroidian mdroidian left a comment

Choose a reason for hiding this comment

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

A few changes requested.

Ideally we test this in the Roam app. You could create a quick command palette function to run it. But we can consider this another task as well.

@sid597
Copy link
Collaborator Author

sid597 commented Apr 4, 2025

@coderabbitai review

Copy link
Contributor

coderabbitai bot commented Apr 4, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
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.

Actionable comments posted: 1

🧹 Nitpick comments (4)
apps/website/app/utils/llm/cors.ts (1)

3-3: Define origins in environment variables for flexibility.

The hardcoded allowed origins could be moved to environment variables to make the application more configurable across different environments without code changes.

-const allowedOrigins = ["https://roamresearch.com", "http://localhost:3000"];
+const allowedOrigins = process.env.ALLOWED_ORIGINS 
+  ? process.env.ALLOWED_ORIGINS.split(",") 
+  : ["https://roamresearch.com", "http://localhost:3000"];
apps/website/app/utils/llm/handler.ts (1)

55-71: Enhance error message with more details for debugging.

The error handling could provide more specific details about the type of API error for better debugging.

if (!response.ok) {
  console.error(`API error:`, responseData);
  return cors(
    request,
    new Response(
      JSON.stringify({
        error: `API error: ${getNestedProperty(responseData, config.errorMessagePath) || "Unknown error"}`,
+       statusCode: response.status,
+       provider: config.apiKeyEnvVar.split('_')[0].toLowerCase()
      }),
      {
        status: response.status,
        headers: { "Content-Type": CONTENT_TYPE_JSON },
      },
    ),
  );
}
apps/website/app/types/llm.ts (2)

1-4: Consider using more specific role types.

The Message type's role property could be more strictly typed for better type safety and autocomplete support.

export type Message = {
-  role: string;
+  role: 'user' | 'assistant' | 'system';
  content: string;
};

16-20: Consider renaming 'documents' field for clarity.

The documents field in RequestBody is actually an array of messages. The naming could be more intuitive.

export type RequestBody = {
-  documents: Message[];
+  messages: Message[];
  passphrase?: string;
  settings: Settings;
};
📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 2b99736 and c2a2c91.

📒 Files selected for processing (8)
  • apps/website/app/api/llm/anthropic/chat/route.ts (1 hunks)
  • apps/website/app/api/llm/gemini/chat/route.ts (1 hunks)
  • apps/website/app/api/llm/openai/chat/route.ts (1 hunks)
  • apps/website/app/types/llm.ts (1 hunks)
  • apps/website/app/utils/llm/cors.ts (1 hunks)
  • apps/website/app/utils/llm/handler.ts (1 hunks)
  • apps/website/app/utils/llm/providers.ts (1 hunks)
  • apps/website/next.config.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
  • apps/website/next.config.ts
  • apps/website/app/api/llm/openai/chat/route.ts
  • apps/website/app/api/llm/anthropic/chat/route.ts
  • apps/website/app/api/llm/gemini/chat/route.ts
🧰 Additional context used
🧬 Code Definitions (2)
apps/website/app/utils/llm/providers.ts (1)
apps/website/app/types/llm.ts (3)
  • LLMProviderConfig (25-32)
  • Message (1-4)
  • Settings (6-14)
apps/website/app/utils/llm/handler.ts (2)
apps/website/app/types/llm.ts (4)
  • LLMProviderConfig (25-32)
  • RequestBody (16-20)
  • CONTENT_TYPE_JSON (22-22)
  • CONTENT_TYPE_TEXT (23-23)
apps/website/app/utils/llm/cors.ts (1)
  • cors (18-48)
🔇 Additional comments (14)
apps/website/app/utils/llm/cors.ts (3)

5-8: LGTM - Vercel preview URL detection.

The isVercelPreviewUrl function properly checks for Vercel preview URLs, which is important for testing the application in Vercel's preview environments.


10-16: LGTM - Origin validation function.

The isAllowedOrigin function correctly checks if the origin is in the allowed list or is a Vercel preview URL, which is important for CORS security.


18-48: LGTM - Well-implemented CORS handling.

The CORS implementation is secure and handles both preflight (OPTIONS) and regular requests appropriately. It only sets CORS headers for allowed origins and includes the necessary headers for secure cross-origin requests.

apps/website/app/utils/llm/handler.ts (6)

10-14: LGTM - Utility function for accessing nested properties.

The getNestedProperty function safely accesses nested properties in objects, which is useful for extracting error messages from API responses.


16-40: LGTM - API key validation and error handling.

The initial part of the handleLLMRequest function correctly extracts request data and validates the presence of an API key, returning appropriate error responses when keys aren't configured.


73-90: LGTM - Handling invalid response formats.

This section correctly handles cases where the API response doesn't contain the expected text format, returning an appropriate error message.


92-112: LGTM - Response and error handling.

The completion of the handleLLMRequest function properly returns the extracted text with correct content type headers and includes comprehensive error handling for exceptions.


115-119: LGTM - Options request handler.

The handleOptionsRequest function correctly handles OPTIONS requests for CORS preflight with a 204 status code.


42-53:

❓ Verification inconclusive

Security concern with API key in URL for Gemini.

When the API URL is a function (as in the Gemini config), there's potential for exposing the API key in the URL if the function embeds it.


🏁 Script executed:

#!/bin/bash
# Check if Gemini config places API key in URL
grep -A 5 "apiUrl.*function" apps/website/app/utils/llm/providers.ts

Length of output: 70


Action Required: Verify API Key Exposure in Gemini Config Function Implementation

The initial concern was that when config.apiUrl is a function (as in the Gemini config), there’s a potential for the API key to be embedded in the URL. Our preliminary grep check in apps/website/app/utils/llm/providers.ts did not reveal any evidence of a function embedding the API key in the URL. However, since the verification output was empty, I recommend a manual review of the Gemini configuration to ensure that:

  • The function (if present) that computes apiUrl does not concatenate or embed the API key into the URL.
  • No insecure URL construction is occurring that might expose the API key in network requests.

Please confirm that the Gemini provider’s implementation in apps/website/app/utils/llm/providers.ts safely handles the API key.

apps/website/app/types/llm.ts (3)

6-14: LGTM - Comprehensive settings type.

The Settings type includes all necessary parameters for configuring LLM requests, including model selection, token limits, temperature, and safety settings.


22-23: LGTM - Content type constants.

These content type constants are correctly defined and will ensure consistent content type headers across the application.


25-32: LGTM - Well-structured provider configuration type.

The LLMProviderConfig type is well-designed and provides all the necessary configuration options to support different LLM providers with their unique APIs.

apps/website/app/utils/llm/providers.ts (2)

3-19: LGTM - OpenAI configuration.

The OpenAI configuration is well-structured and correctly implements the API interface, including authentication, request formatting, and response handling.


44-60: LGTM - Anthropic configuration.

The Anthropic configuration is well-structured and correctly implements the API interface, including appropriate headers with the API version.

Copy link
Contributor

@mdroidian mdroidian left a comment

Choose a reason for hiding this comment

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

Mostly nits.

"tasks": {
"build": {
"passThroughEnv": ["RESEND_API_KEY"],
"passThroughEnv": [
Copy link
Contributor

Choose a reason for hiding this comment

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

What new build script command would we need to add?

Copy link
Contributor

@mdroidian mdroidian left a comment

Choose a reason for hiding this comment

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

👍

@mdroidian mdroidian merged commit d52100a into main Apr 10, 2025
3 checks passed
@mdroidian mdroidian deleted the move-api-endpoints-to-serverless branch April 10, 2025 19:55
@github-project-automation github-project-automation bot moved this to Done in General Apr 10, 2025
mdroidian pushed a commit that referenced this pull request May 16, 2025
author Trang Doan <[email protected]> 1744314525 -0400
committer Michael Gartner <[email protected]> 1747354088 -0600

ENG-96 Create new relationship between nodes (#115)

* instantiate new relationship worked

* fix

* address PR comments

* fix bi-directional update issues

* show only compatible node type options

* small fix

* breakdown the components. use datacore

* working

* address PR comments

* improve search by only allowing compatible node results

* .

* rm dataview

---------

Co-authored-by: Michael Gartner <[email protected]>

Move llm-api endpoints to vercel serverless (#102)

* testing gemini

* move endgoint to website

* open ai endpoint

* added anthropic endpoint

* pass env vars

* add cors handdling and options

* .

* using centralised cors middleware

* only adding bypass cookie

* use right key

* remove the bypass token requirement

* sanitize, fix routes

* remove server action config

* DRY

* remove unused

* address review

* adress review

Roam: Add feedback toggle (#118)

* add settings to hide or show button, also works when disabled or enabled midway

* review

* .

---------

Co-authored-by: Michael Gartner <[email protected]>

[ENG-197] Fix creating link with invalid chars (#121)

* fix creating link with invalid chars

* placeholder update

---------

Co-authored-by: Michael Gartner <[email protected]>

Roam:  Add feedback button to settings menu - ENG-147 (#122)

* add button to bottom right, don't hide sdk css, tested

* remove intent not working
git

* remove ts-ignore and use a better type def

* remove styling

Update NodeConfig to use new UIDs for DiscourseNodeIndex and DiscourseNodeSpecification components (#126)

Roam: Add PostHog user identification for enhanced analytics tracking using user's roam UID as the unique identifier - ENG-177 (#123)

* add posthog identify

* remove username and email to keep it anonymus

* double userUid and best practice for js

Roam: Discourse Context Overlay - remove queue and arbitrary delay (#127)

* Refactor getOverlayInfo to use async/await and improve error handling. Update cache key from title to tag and remove overlayQueue logic for cleaner implementation.

* Remove experimental getOverlayInfo function

* Remove unused refreshUi logic

[ENG-44] Display relations (#116)

* instantiate new relationship worked

* add display relations

* remove dv

* sm fix

[ENG-198] Filtered out related file in search (#125)

* filtered out related file

* fix some naming

[ENG-97] Use TailwindCSS in obsidian app (#128)

* Update Obsidian app to integrate Tailwind CSS with PostCSS and Autoprefixer support

- Added Tailwind CSS, PostCSS, and Autoprefixer to package dependencies
- Configured styles.css to include Tailwind directives
- Enhanced compile script to process styles using PostCSS with Tailwind and Autoprefixer

* delete irrelevant package

[ENG-192] Change all existing styles to using tw (#129)

* Update Obsidian app to integrate Tailwind CSS with PostCSS and Autoprefixer support

- Added Tailwind CSS, PostCSS, and Autoprefixer to package dependencies
- Configured styles.css to include Tailwind directives
- Enhanced compile script to process styles using PostCSS with Tailwind and Autoprefixer

* delete irrelevant package

* changing all styles to tailwindcss

* Update Obsidian app to integrate Tailwind CSS with PostCSS and Autoprefixer support

- Added Tailwind CSS, PostCSS, and Autoprefixer to package dependencies
- Configured styles.css to include Tailwind directives
- Enhanced compile script to process styles using PostCSS with Tailwind and Autoprefixer

* delete irrelevant package

* changing all styles to tailwindcss

---------

Co-authored-by: Michael Gartner <[email protected]>

Roam: Bug-fix: Don't let user create discourse nodes with empty text using node context menu - ENG-171 (#130)

* functional covering all three cases tested locally

* apply coderabbit review suggestion

* better approach one that I understand and can reason about

* accidental removal of onClose

Update Roam app version to 0.13.0 in package.json and package-lock.json (#134)

[ENG-204] Move from localStorage to extensionAPI.settings (#133)

* cur progress

* address PR comments

* kinda works. need to test more

* small fix

* address PR comments

.

Create publish-obsidian.yml

Update publish-obsidian.yml

Update publish-obsidian.yml

Update publish-obsidian.yml

Enhance DiscourseContextOverlay: Update button styles to include loading state and improve score/ref display during loading. Use placeholders for score and refs when loading. (#136)

.

Update publish-obsidian.yml

Update publish-obsidian.yml

Update publish-obsidian.yml

Update publish-obsidian.yml

Refactor ExportDialog: Remove discourseGraphEnabled state and simplify FormGroup visibility logic. Set includeDiscourseContext to false by default. (#139)

Enhance LabelDialog: Add confirmText to return object for improved button text handling based on action type. (#141)

Additional styles / cursor rules (#142)

* Update STYLE_GUIDE.md and main.mdc: Add guideline for utilizing utility functions for reusable logic and common operations.

* Update STYLE_GUIDE.md and main.mdc: Add guideline to prefer arrow functions over regular function declarations.

* Update main.mdc: Add guideline to prefer Tailwind classes when refactoring inline styles.

* Update STYLE_GUIDE.md and main.mdc: Add guideline to prefer early returns over nested conditionals for improved readability.

Roam: When a user deletes a node also delete all the corresponding relations to the node - ENG-26 (#149)

* ask user for confirmation, delete corresponding relations

* address review

* address review

* address comments

[ENG-301] Create node in right-click menu (#152)

* create node in right-click menu

* small fix

* address PR comments

* address PR comments

add readme and remove sample commands

remove sample editor command

rm space

minor fixes

Roam: Bug fix - Insert Discourse Node after creation (#154)

* remove focus after menu select to allow updateBlock to work

* add clarifying comment

[ENG-308] Add command to open DG settings (#158)

* add command to open DG settings

* edit comment

ENG-322 - Switch from MIT to Apache 2.0 license (#156)

* Switch from MIT to Apache 2.0 license

* copyright discourse graphs

* rm liscense from apps/roam

---------

Co-authored-by: Michael Gartner <[email protected]>

initial port

[ENG-207] Move Github sync setting to individual nodes (#124)

* current progress

* improve in UI: if sync is turned off then also turn off the comments configuration

* address PR comments

* revert graphOverviewUid bug

* revert graphOverviewUid bug - getDiscourseNodes

* avoid racing conditions for github sync

* nested settings

* temp fix to race condition

* remove unecessary DOM and match existing styles

---------

Co-authored-by: Michael Gartner <[email protected]>

Eng 286 show when GitHub sync is disabled globally (#143)

* Refactor GitHub Sync settings in NodeConfig and GeneralSettings components

- Updated the onChange handler for GitHub Sync to use async/await and added a timeout for refreshing the config tree.
- Introduced a global settings check in NodeConfig to conditionally render the GitHub Sync checkbox and comments configuration.
- Passed setMainTab prop to NodeConfig for better navigation control.

This improves the user experience by ensuring that settings are updated correctly and provides clear feedback when global settings are disabled.

* matchingNode fix

.

Refactor Export components to use getSetting for consistent settings retrieval

- Updated ExportDialog and ExportGithub components to replace localStorageGet with getSetting for fetching GitHub OAuth and repository settings.
- Modified extensionSettings utility functions to use arrow functions and provide a default value for getSetting.
- Improved code readability and maintainability by standardizing the method of accessing settings.

Eng 286 show when GitHub sync is disabled globally (#143)

* Refactor GitHub Sync settings in NodeConfig and GeneralSettings components

- Updated the onChange handler for GitHub Sync to use async/await and added a timeout for refreshing the config tree.
- Introduced a global settings check in NodeConfig to conditionally render the GitHub Sync checkbox and comments configuration.
- Passed setMainTab prop to NodeConfig for better navigation control.

This improves the user experience by ensuring that settings are updated correctly and provides clear feedback when global settings are disabled.

* matchingNode fix

.
mdroidian pushed a commit that referenced this pull request May 16, 2025
author Trang Doan <[email protected]> 1744314525 -0400
committer Michael Gartner <[email protected]> 1747354088 -0600

ENG-96 Create new relationship between nodes (#115)

* instantiate new relationship worked

* fix

* address PR comments

* fix bi-directional update issues

* show only compatible node type options

* small fix

* breakdown the components. use datacore

* working

* address PR comments

* improve search by only allowing compatible node results

* .

* rm dataview

---------

Co-authored-by: Michael Gartner <[email protected]>

Move llm-api endpoints to vercel serverless (#102)

* testing gemini

* move endgoint to website

* open ai endpoint

* added anthropic endpoint

* pass env vars

* add cors handdling and options

* .

* using centralised cors middleware

* only adding bypass cookie

* use right key

* remove the bypass token requirement

* sanitize, fix routes

* remove server action config

* DRY

* remove unused

* address review

* adress review

Roam: Add feedback toggle (#118)

* add settings to hide or show button, also works when disabled or enabled midway

* review

* .

---------

Co-authored-by: Michael Gartner <[email protected]>

[ENG-197] Fix creating link with invalid chars (#121)

* fix creating link with invalid chars

* placeholder update

---------

Co-authored-by: Michael Gartner <[email protected]>

Roam:  Add feedback button to settings menu - ENG-147 (#122)

* add button to bottom right, don't hide sdk css, tested

* remove intent not working
git

* remove ts-ignore and use a better type def

* remove styling

Update NodeConfig to use new UIDs for DiscourseNodeIndex and DiscourseNodeSpecification components (#126)

Roam: Add PostHog user identification for enhanced analytics tracking using user's roam UID as the unique identifier - ENG-177 (#123)

* add posthog identify

* remove username and email to keep it anonymus

* double userUid and best practice for js

Roam: Discourse Context Overlay - remove queue and arbitrary delay (#127)

* Refactor getOverlayInfo to use async/await and improve error handling. Update cache key from title to tag and remove overlayQueue logic for cleaner implementation.

* Remove experimental getOverlayInfo function

* Remove unused refreshUi logic

[ENG-44] Display relations (#116)

* instantiate new relationship worked

* add display relations

* remove dv

* sm fix

[ENG-198] Filtered out related file in search (#125)

* filtered out related file

* fix some naming

[ENG-97] Use TailwindCSS in obsidian app (#128)

* Update Obsidian app to integrate Tailwind CSS with PostCSS and Autoprefixer support

- Added Tailwind CSS, PostCSS, and Autoprefixer to package dependencies
- Configured styles.css to include Tailwind directives
- Enhanced compile script to process styles using PostCSS with Tailwind and Autoprefixer

* delete irrelevant package

[ENG-192] Change all existing styles to using tw (#129)

* Update Obsidian app to integrate Tailwind CSS with PostCSS and Autoprefixer support

- Added Tailwind CSS, PostCSS, and Autoprefixer to package dependencies
- Configured styles.css to include Tailwind directives
- Enhanced compile script to process styles using PostCSS with Tailwind and Autoprefixer

* delete irrelevant package

* changing all styles to tailwindcss

* Update Obsidian app to integrate Tailwind CSS with PostCSS and Autoprefixer support

- Added Tailwind CSS, PostCSS, and Autoprefixer to package dependencies
- Configured styles.css to include Tailwind directives
- Enhanced compile script to process styles using PostCSS with Tailwind and Autoprefixer

* delete irrelevant package

* changing all styles to tailwindcss

---------

Co-authored-by: Michael Gartner <[email protected]>

Roam: Bug-fix: Don't let user create discourse nodes with empty text using node context menu - ENG-171 (#130)

* functional covering all three cases tested locally

* apply coderabbit review suggestion

* better approach one that I understand and can reason about

* accidental removal of onClose

Update Roam app version to 0.13.0 in package.json and package-lock.json (#134)

[ENG-204] Move from localStorage to extensionAPI.settings (#133)

* cur progress

* address PR comments

* kinda works. need to test more

* small fix

* address PR comments

.

Create publish-obsidian.yml

Update publish-obsidian.yml

Update publish-obsidian.yml

Update publish-obsidian.yml

Enhance DiscourseContextOverlay: Update button styles to include loading state and improve score/ref display during loading. Use placeholders for score and refs when loading. (#136)

.

Update publish-obsidian.yml

Update publish-obsidian.yml

Update publish-obsidian.yml

Update publish-obsidian.yml

Refactor ExportDialog: Remove discourseGraphEnabled state and simplify FormGroup visibility logic. Set includeDiscourseContext to false by default. (#139)

Enhance LabelDialog: Add confirmText to return object for improved button text handling based on action type. (#141)

Additional styles / cursor rules (#142)

* Update STYLE_GUIDE.md and main.mdc: Add guideline for utilizing utility functions for reusable logic and common operations.

* Update STYLE_GUIDE.md and main.mdc: Add guideline to prefer arrow functions over regular function declarations.

* Update main.mdc: Add guideline to prefer Tailwind classes when refactoring inline styles.

* Update STYLE_GUIDE.md and main.mdc: Add guideline to prefer early returns over nested conditionals for improved readability.

Roam: When a user deletes a node also delete all the corresponding relations to the node - ENG-26 (#149)

* ask user for confirmation, delete corresponding relations

* address review

* address review

* address comments

[ENG-301] Create node in right-click menu (#152)

* create node in right-click menu

* small fix

* address PR comments

* address PR comments

add readme and remove sample commands

remove sample editor command

rm space

minor fixes

Roam: Bug fix - Insert Discourse Node after creation (#154)

* remove focus after menu select to allow updateBlock to work

* add clarifying comment

[ENG-308] Add command to open DG settings (#158)

* add command to open DG settings

* edit comment

ENG-322 - Switch from MIT to Apache 2.0 license (#156)

* Switch from MIT to Apache 2.0 license

* copyright discourse graphs

* rm liscense from apps/roam

---------

Co-authored-by: Michael Gartner <[email protected]>

initial port

[ENG-207] Move Github sync setting to individual nodes (#124)

* current progress

* improve in UI: if sync is turned off then also turn off the comments configuration

* address PR comments

* revert graphOverviewUid bug

* revert graphOverviewUid bug - getDiscourseNodes

* avoid racing conditions for github sync

* nested settings

* temp fix to race condition

* remove unecessary DOM and match existing styles

---------

Co-authored-by: Michael Gartner <[email protected]>

Eng 286 show when GitHub sync is disabled globally (#143)

* Refactor GitHub Sync settings in NodeConfig and GeneralSettings components

- Updated the onChange handler for GitHub Sync to use async/await and added a timeout for refreshing the config tree.
- Introduced a global settings check in NodeConfig to conditionally render the GitHub Sync checkbox and comments configuration.
- Passed setMainTab prop to NodeConfig for better navigation control.

This improves the user experience by ensuring that settings are updated correctly and provides clear feedback when global settings are disabled.

* matchingNode fix

.

Refactor Export components to use getSetting for consistent settings retrieval

- Updated ExportDialog and ExportGithub components to replace localStorageGet with getSetting for fetching GitHub OAuth and repository settings.
- Modified extensionSettings utility functions to use arrow functions and provide a default value for getSetting.
- Improved code readability and maintainability by standardizing the method of accessing settings.

Eng 286 show when GitHub sync is disabled globally (#143)

* Refactor GitHub Sync settings in NodeConfig and GeneralSettings components

- Updated the onChange handler for GitHub Sync to use async/await and added a timeout for refreshing the config tree.
- Introduced a global settings check in NodeConfig to conditionally render the GitHub Sync checkbox and comments configuration.
- Passed setMainTab prop to NodeConfig for better navigation control.

This improves the user experience by ensuring that settings are updated correctly and provides clear feedback when global settings are disabled.

* matchingNode fix

.
mdroidian pushed a commit that referenced this pull request Jun 3, 2025
author Trang Doan <[email protected]> 1744314525 -0400
committer Michael Gartner <[email protected]> 1747354088 -0600

ENG-96 Create new relationship between nodes (#115)

* instantiate new relationship worked

* fix

* address PR comments

* fix bi-directional update issues

* show only compatible node type options

* small fix

* breakdown the components. use datacore

* working

* address PR comments

* improve search by only allowing compatible node results

* .

* rm dataview

---------

Co-authored-by: Michael Gartner <[email protected]>

Move llm-api endpoints to vercel serverless (#102)

* testing gemini

* move endgoint to website

* open ai endpoint

* added anthropic endpoint

* pass env vars

* add cors handdling and options

* .

* using centralised cors middleware

* only adding bypass cookie

* use right key

* remove the bypass token requirement

* sanitize, fix routes

* remove server action config

* DRY

* remove unused

* address review

* adress review

Roam: Add feedback toggle (#118)

* add settings to hide or show button, also works when disabled or enabled midway

* review

* .

---------

Co-authored-by: Michael Gartner <[email protected]>

[ENG-197] Fix creating link with invalid chars (#121)

* fix creating link with invalid chars

* placeholder update

---------

Co-authored-by: Michael Gartner <[email protected]>

Roam:  Add feedback button to settings menu - ENG-147 (#122)

* add button to bottom right, don't hide sdk css, tested

* remove intent not working
git

* remove ts-ignore and use a better type def

* remove styling

Update NodeConfig to use new UIDs for DiscourseNodeIndex and DiscourseNodeSpecification components (#126)

Roam: Add PostHog user identification for enhanced analytics tracking using user's roam UID as the unique identifier - ENG-177 (#123)

* add posthog identify

* remove username and email to keep it anonymus

* double userUid and best practice for js

Roam: Discourse Context Overlay - remove queue and arbitrary delay (#127)

* Refactor getOverlayInfo to use async/await and improve error handling. Update cache key from title to tag and remove overlayQueue logic for cleaner implementation.

* Remove experimental getOverlayInfo function

* Remove unused refreshUi logic

[ENG-44] Display relations (#116)

* instantiate new relationship worked

* add display relations

* remove dv

* sm fix

[ENG-198] Filtered out related file in search (#125)

* filtered out related file

* fix some naming

[ENG-97] Use TailwindCSS in obsidian app (#128)

* Update Obsidian app to integrate Tailwind CSS with PostCSS and Autoprefixer support

- Added Tailwind CSS, PostCSS, and Autoprefixer to package dependencies
- Configured styles.css to include Tailwind directives
- Enhanced compile script to process styles using PostCSS with Tailwind and Autoprefixer

* delete irrelevant package

[ENG-192] Change all existing styles to using tw (#129)

* Update Obsidian app to integrate Tailwind CSS with PostCSS and Autoprefixer support

- Added Tailwind CSS, PostCSS, and Autoprefixer to package dependencies
- Configured styles.css to include Tailwind directives
- Enhanced compile script to process styles using PostCSS with Tailwind and Autoprefixer

* delete irrelevant package

* changing all styles to tailwindcss

* Update Obsidian app to integrate Tailwind CSS with PostCSS and Autoprefixer support

- Added Tailwind CSS, PostCSS, and Autoprefixer to package dependencies
- Configured styles.css to include Tailwind directives
- Enhanced compile script to process styles using PostCSS with Tailwind and Autoprefixer

* delete irrelevant package

* changing all styles to tailwindcss

---------

Co-authored-by: Michael Gartner <[email protected]>

Roam: Bug-fix: Don't let user create discourse nodes with empty text using node context menu - ENG-171 (#130)

* functional covering all three cases tested locally

* apply coderabbit review suggestion

* better approach one that I understand and can reason about

* accidental removal of onClose

Update Roam app version to 0.13.0 in package.json and package-lock.json (#134)

[ENG-204] Move from localStorage to extensionAPI.settings (#133)

* cur progress

* address PR comments

* kinda works. need to test more

* small fix

* address PR comments

.

Create publish-obsidian.yml

Update publish-obsidian.yml

Update publish-obsidian.yml

Update publish-obsidian.yml

Enhance DiscourseContextOverlay: Update button styles to include loading state and improve score/ref display during loading. Use placeholders for score and refs when loading. (#136)

.

Update publish-obsidian.yml

Update publish-obsidian.yml

Update publish-obsidian.yml

Update publish-obsidian.yml

Refactor ExportDialog: Remove discourseGraphEnabled state and simplify FormGroup visibility logic. Set includeDiscourseContext to false by default. (#139)

Enhance LabelDialog: Add confirmText to return object for improved button text handling based on action type. (#141)

Additional styles / cursor rules (#142)

* Update STYLE_GUIDE.md and main.mdc: Add guideline for utilizing utility functions for reusable logic and common operations.

* Update STYLE_GUIDE.md and main.mdc: Add guideline to prefer arrow functions over regular function declarations.

* Update main.mdc: Add guideline to prefer Tailwind classes when refactoring inline styles.

* Update STYLE_GUIDE.md and main.mdc: Add guideline to prefer early returns over nested conditionals for improved readability.

Roam: When a user deletes a node also delete all the corresponding relations to the node - ENG-26 (#149)

* ask user for confirmation, delete corresponding relations

* address review

* address review

* address comments

[ENG-301] Create node in right-click menu (#152)

* create node in right-click menu

* small fix

* address PR comments

* address PR comments

add readme and remove sample commands

remove sample editor command

rm space

minor fixes

Roam: Bug fix - Insert Discourse Node after creation (#154)

* remove focus after menu select to allow updateBlock to work

* add clarifying comment

[ENG-308] Add command to open DG settings (#158)

* add command to open DG settings

* edit comment

ENG-322 - Switch from MIT to Apache 2.0 license (#156)

* Switch from MIT to Apache 2.0 license

* copyright discourse graphs

* rm liscense from apps/roam

---------

Co-authored-by: Michael Gartner <[email protected]>

initial port

[ENG-207] Move Github sync setting to individual nodes (#124)

* current progress

* improve in UI: if sync is turned off then also turn off the comments configuration

* address PR comments

* revert graphOverviewUid bug

* revert graphOverviewUid bug - getDiscourseNodes

* avoid racing conditions for github sync

* nested settings

* temp fix to race condition

* remove unecessary DOM and match existing styles

---------

Co-authored-by: Michael Gartner <[email protected]>

Eng 286 show when GitHub sync is disabled globally (#143)

* Refactor GitHub Sync settings in NodeConfig and GeneralSettings components

- Updated the onChange handler for GitHub Sync to use async/await and added a timeout for refreshing the config tree.
- Introduced a global settings check in NodeConfig to conditionally render the GitHub Sync checkbox and comments configuration.
- Passed setMainTab prop to NodeConfig for better navigation control.

This improves the user experience by ensuring that settings are updated correctly and provides clear feedback when global settings are disabled.

* matchingNode fix

.

Refactor Export components to use getSetting for consistent settings retrieval

- Updated ExportDialog and ExportGithub components to replace localStorageGet with getSetting for fetching GitHub OAuth and repository settings.
- Modified extensionSettings utility functions to use arrow functions and provide a default value for getSetting.
- Improved code readability and maintainability by standardizing the method of accessing settings.

Eng 286 show when GitHub sync is disabled globally (#143)

* Refactor GitHub Sync settings in NodeConfig and GeneralSettings components

- Updated the onChange handler for GitHub Sync to use async/await and added a timeout for refreshing the config tree.
- Introduced a global settings check in NodeConfig to conditionally render the GitHub Sync checkbox and comments configuration.
- Passed setMainTab prop to NodeConfig for better navigation control.

This improves the user experience by ensuring that settings are updated correctly and provides clear feedback when global settings are disabled.

* matchingNode fix

.
mdroidian pushed a commit that referenced this pull request Jun 9, 2025
author Trang Doan <[email protected]> 1744314525 -0400
committer Michael Gartner <[email protected]> 1747354088 -0600

ENG-96 Create new relationship between nodes (#115)

* instantiate new relationship worked

* fix

* address PR comments

* fix bi-directional update issues

* show only compatible node type options

* small fix

* breakdown the components. use datacore

* working

* address PR comments

* improve search by only allowing compatible node results

* .

* rm dataview

---------

Co-authored-by: Michael Gartner <[email protected]>

Move llm-api endpoints to vercel serverless (#102)

* testing gemini

* move endgoint to website

* open ai endpoint

* added anthropic endpoint

* pass env vars

* add cors handdling and options

* .

* using centralised cors middleware

* only adding bypass cookie

* use right key

* remove the bypass token requirement

* sanitize, fix routes

* remove server action config

* DRY

* remove unused

* address review

* adress review

Roam: Add feedback toggle (#118)

* add settings to hide or show button, also works when disabled or enabled midway

* review

* .

---------

Co-authored-by: Michael Gartner <[email protected]>

[ENG-197] Fix creating link with invalid chars (#121)

* fix creating link with invalid chars

* placeholder update

---------

Co-authored-by: Michael Gartner <[email protected]>

Roam:  Add feedback button to settings menu - ENG-147 (#122)

* add button to bottom right, don't hide sdk css, tested

* remove intent not working
git

* remove ts-ignore and use a better type def

* remove styling

Update NodeConfig to use new UIDs for DiscourseNodeIndex and DiscourseNodeSpecification components (#126)

Roam: Add PostHog user identification for enhanced analytics tracking using user's roam UID as the unique identifier - ENG-177 (#123)

* add posthog identify

* remove username and email to keep it anonymus

* double userUid and best practice for js

Roam: Discourse Context Overlay - remove queue and arbitrary delay (#127)

* Refactor getOverlayInfo to use async/await and improve error handling. Update cache key from title to tag and remove overlayQueue logic for cleaner implementation.

* Remove experimental getOverlayInfo function

* Remove unused refreshUi logic

[ENG-44] Display relations (#116)

* instantiate new relationship worked

* add display relations

* remove dv

* sm fix

[ENG-198] Filtered out related file in search (#125)

* filtered out related file

* fix some naming

[ENG-97] Use TailwindCSS in obsidian app (#128)

* Update Obsidian app to integrate Tailwind CSS with PostCSS and Autoprefixer support

- Added Tailwind CSS, PostCSS, and Autoprefixer to package dependencies
- Configured styles.css to include Tailwind directives
- Enhanced compile script to process styles using PostCSS with Tailwind and Autoprefixer

* delete irrelevant package

[ENG-192] Change all existing styles to using tw (#129)

* Update Obsidian app to integrate Tailwind CSS with PostCSS and Autoprefixer support

- Added Tailwind CSS, PostCSS, and Autoprefixer to package dependencies
- Configured styles.css to include Tailwind directives
- Enhanced compile script to process styles using PostCSS with Tailwind and Autoprefixer

* delete irrelevant package

* changing all styles to tailwindcss

* Update Obsidian app to integrate Tailwind CSS with PostCSS and Autoprefixer support

- Added Tailwind CSS, PostCSS, and Autoprefixer to package dependencies
- Configured styles.css to include Tailwind directives
- Enhanced compile script to process styles using PostCSS with Tailwind and Autoprefixer

* delete irrelevant package

* changing all styles to tailwindcss

---------

Co-authored-by: Michael Gartner <[email protected]>

Roam: Bug-fix: Don't let user create discourse nodes with empty text using node context menu - ENG-171 (#130)

* functional covering all three cases tested locally

* apply coderabbit review suggestion

* better approach one that I understand and can reason about

* accidental removal of onClose

Update Roam app version to 0.13.0 in package.json and package-lock.json (#134)

[ENG-204] Move from localStorage to extensionAPI.settings (#133)

* cur progress

* address PR comments

* kinda works. need to test more

* small fix

* address PR comments

.

Create publish-obsidian.yml

Update publish-obsidian.yml

Update publish-obsidian.yml

Update publish-obsidian.yml

Enhance DiscourseContextOverlay: Update button styles to include loading state and improve score/ref display during loading. Use placeholders for score and refs when loading. (#136)

.

Update publish-obsidian.yml

Update publish-obsidian.yml

Update publish-obsidian.yml

Update publish-obsidian.yml

Refactor ExportDialog: Remove discourseGraphEnabled state and simplify FormGroup visibility logic. Set includeDiscourseContext to false by default. (#139)

Enhance LabelDialog: Add confirmText to return object for improved button text handling based on action type. (#141)

Additional styles / cursor rules (#142)

* Update STYLE_GUIDE.md and main.mdc: Add guideline for utilizing utility functions for reusable logic and common operations.

* Update STYLE_GUIDE.md and main.mdc: Add guideline to prefer arrow functions over regular function declarations.

* Update main.mdc: Add guideline to prefer Tailwind classes when refactoring inline styles.

* Update STYLE_GUIDE.md and main.mdc: Add guideline to prefer early returns over nested conditionals for improved readability.

Roam: When a user deletes a node also delete all the corresponding relations to the node - ENG-26 (#149)

* ask user for confirmation, delete corresponding relations

* address review

* address review

* address comments

[ENG-301] Create node in right-click menu (#152)

* create node in right-click menu

* small fix

* address PR comments

* address PR comments

add readme and remove sample commands

remove sample editor command

rm space

minor fixes

Roam: Bug fix - Insert Discourse Node after creation (#154)

* remove focus after menu select to allow updateBlock to work

* add clarifying comment

[ENG-308] Add command to open DG settings (#158)

* add command to open DG settings

* edit comment

ENG-322 - Switch from MIT to Apache 2.0 license (#156)

* Switch from MIT to Apache 2.0 license

* copyright discourse graphs

* rm liscense from apps/roam

---------

Co-authored-by: Michael Gartner <[email protected]>

initial port

[ENG-207] Move Github sync setting to individual nodes (#124)

* current progress

* improve in UI: if sync is turned off then also turn off the comments configuration

* address PR comments

* revert graphOverviewUid bug

* revert graphOverviewUid bug - getDiscourseNodes

* avoid racing conditions for github sync

* nested settings

* temp fix to race condition

* remove unecessary DOM and match existing styles

---------

Co-authored-by: Michael Gartner <[email protected]>

Eng 286 show when GitHub sync is disabled globally (#143)

* Refactor GitHub Sync settings in NodeConfig and GeneralSettings components

- Updated the onChange handler for GitHub Sync to use async/await and added a timeout for refreshing the config tree.
- Introduced a global settings check in NodeConfig to conditionally render the GitHub Sync checkbox and comments configuration.
- Passed setMainTab prop to NodeConfig for better navigation control.

This improves the user experience by ensuring that settings are updated correctly and provides clear feedback when global settings are disabled.

* matchingNode fix

.

Refactor Export components to use getSetting for consistent settings retrieval

- Updated ExportDialog and ExportGithub components to replace localStorageGet with getSetting for fetching GitHub OAuth and repository settings.
- Modified extensionSettings utility functions to use arrow functions and provide a default value for getSetting.
- Improved code readability and maintainability by standardizing the method of accessing settings.

Eng 286 show when GitHub sync is disabled globally (#143)

* Refactor GitHub Sync settings in NodeConfig and GeneralSettings components

- Updated the onChange handler for GitHub Sync to use async/await and added a timeout for refreshing the config tree.
- Introduced a global settings check in NodeConfig to conditionally render the GitHub Sync checkbox and comments configuration.
- Passed setMainTab prop to NodeConfig for better navigation control.

This improves the user experience by ensuring that settings are updated correctly and provides clear feedback when global settings are disabled.

* matchingNode fix

.
mdroidian pushed a commit that referenced this pull request Jun 14, 2025
author Trang Doan <[email protected]> 1744314525 -0400
committer Michael Gartner <[email protected]> 1747354088 -0600

ENG-96 Create new relationship between nodes (#115)

* instantiate new relationship worked

* fix

* address PR comments

* fix bi-directional update issues

* show only compatible node type options

* small fix

* breakdown the components. use datacore

* working

* address PR comments

* improve search by only allowing compatible node results

* .

* rm dataview

---------

Co-authored-by: Michael Gartner <[email protected]>

Move llm-api endpoints to vercel serverless (#102)

* testing gemini

* move endgoint to website

* open ai endpoint

* added anthropic endpoint

* pass env vars

* add cors handdling and options

* .

* using centralised cors middleware

* only adding bypass cookie

* use right key

* remove the bypass token requirement

* sanitize, fix routes

* remove server action config

* DRY

* remove unused

* address review

* adress review

Roam: Add feedback toggle (#118)

* add settings to hide or show button, also works when disabled or enabled midway

* review

* .

---------

Co-authored-by: Michael Gartner <[email protected]>

[ENG-197] Fix creating link with invalid chars (#121)

* fix creating link with invalid chars

* placeholder update

---------

Co-authored-by: Michael Gartner <[email protected]>

Roam:  Add feedback button to settings menu - ENG-147 (#122)

* add button to bottom right, don't hide sdk css, tested

* remove intent not working
git

* remove ts-ignore and use a better type def

* remove styling

Update NodeConfig to use new UIDs for DiscourseNodeIndex and DiscourseNodeSpecification components (#126)

Roam: Add PostHog user identification for enhanced analytics tracking using user's roam UID as the unique identifier - ENG-177 (#123)

* add posthog identify

* remove username and email to keep it anonymus

* double userUid and best practice for js

Roam: Discourse Context Overlay - remove queue and arbitrary delay (#127)

* Refactor getOverlayInfo to use async/await and improve error handling. Update cache key from title to tag and remove overlayQueue logic for cleaner implementation.

* Remove experimental getOverlayInfo function

* Remove unused refreshUi logic

[ENG-44] Display relations (#116)

* instantiate new relationship worked

* add display relations

* remove dv

* sm fix

[ENG-198] Filtered out related file in search (#125)

* filtered out related file

* fix some naming

[ENG-97] Use TailwindCSS in obsidian app (#128)

* Update Obsidian app to integrate Tailwind CSS with PostCSS and Autoprefixer support

- Added Tailwind CSS, PostCSS, and Autoprefixer to package dependencies
- Configured styles.css to include Tailwind directives
- Enhanced compile script to process styles using PostCSS with Tailwind and Autoprefixer

* delete irrelevant package

[ENG-192] Change all existing styles to using tw (#129)

* Update Obsidian app to integrate Tailwind CSS with PostCSS and Autoprefixer support

- Added Tailwind CSS, PostCSS, and Autoprefixer to package dependencies
- Configured styles.css to include Tailwind directives
- Enhanced compile script to process styles using PostCSS with Tailwind and Autoprefixer

* delete irrelevant package

* changing all styles to tailwindcss

* Update Obsidian app to integrate Tailwind CSS with PostCSS and Autoprefixer support

- Added Tailwind CSS, PostCSS, and Autoprefixer to package dependencies
- Configured styles.css to include Tailwind directives
- Enhanced compile script to process styles using PostCSS with Tailwind and Autoprefixer

* delete irrelevant package

* changing all styles to tailwindcss

---------

Co-authored-by: Michael Gartner <[email protected]>

Roam: Bug-fix: Don't let user create discourse nodes with empty text using node context menu - ENG-171 (#130)

* functional covering all three cases tested locally

* apply coderabbit review suggestion

* better approach one that I understand and can reason about

* accidental removal of onClose

Update Roam app version to 0.13.0 in package.json and package-lock.json (#134)

[ENG-204] Move from localStorage to extensionAPI.settings (#133)

* cur progress

* address PR comments

* kinda works. need to test more

* small fix

* address PR comments

.

Create publish-obsidian.yml

Update publish-obsidian.yml

Update publish-obsidian.yml

Update publish-obsidian.yml

Enhance DiscourseContextOverlay: Update button styles to include loading state and improve score/ref display during loading. Use placeholders for score and refs when loading. (#136)

.

Update publish-obsidian.yml

Update publish-obsidian.yml

Update publish-obsidian.yml

Update publish-obsidian.yml

Refactor ExportDialog: Remove discourseGraphEnabled state and simplify FormGroup visibility logic. Set includeDiscourseContext to false by default. (#139)

Enhance LabelDialog: Add confirmText to return object for improved button text handling based on action type. (#141)

Additional styles / cursor rules (#142)

* Update STYLE_GUIDE.md and main.mdc: Add guideline for utilizing utility functions for reusable logic and common operations.

* Update STYLE_GUIDE.md and main.mdc: Add guideline to prefer arrow functions over regular function declarations.

* Update main.mdc: Add guideline to prefer Tailwind classes when refactoring inline styles.

* Update STYLE_GUIDE.md and main.mdc: Add guideline to prefer early returns over nested conditionals for improved readability.

Roam: When a user deletes a node also delete all the corresponding relations to the node - ENG-26 (#149)

* ask user for confirmation, delete corresponding relations

* address review

* address review

* address comments

[ENG-301] Create node in right-click menu (#152)

* create node in right-click menu

* small fix

* address PR comments

* address PR comments

add readme and remove sample commands

remove sample editor command

rm space

minor fixes

Roam: Bug fix - Insert Discourse Node after creation (#154)

* remove focus after menu select to allow updateBlock to work

* add clarifying comment

[ENG-308] Add command to open DG settings (#158)

* add command to open DG settings

* edit comment

ENG-322 - Switch from MIT to Apache 2.0 license (#156)

* Switch from MIT to Apache 2.0 license

* copyright discourse graphs

* rm liscense from apps/roam

---------

Co-authored-by: Michael Gartner <[email protected]>

initial port

[ENG-207] Move Github sync setting to individual nodes (#124)

* current progress

* improve in UI: if sync is turned off then also turn off the comments configuration

* address PR comments

* revert graphOverviewUid bug

* revert graphOverviewUid bug - getDiscourseNodes

* avoid racing conditions for github sync

* nested settings

* temp fix to race condition

* remove unecessary DOM and match existing styles

---------

Co-authored-by: Michael Gartner <[email protected]>

Eng 286 show when GitHub sync is disabled globally (#143)

* Refactor GitHub Sync settings in NodeConfig and GeneralSettings components

- Updated the onChange handler for GitHub Sync to use async/await and added a timeout for refreshing the config tree.
- Introduced a global settings check in NodeConfig to conditionally render the GitHub Sync checkbox and comments configuration.
- Passed setMainTab prop to NodeConfig for better navigation control.

This improves the user experience by ensuring that settings are updated correctly and provides clear feedback when global settings are disabled.

* matchingNode fix

.

Refactor Export components to use getSetting for consistent settings retrieval

- Updated ExportDialog and ExportGithub components to replace localStorageGet with getSetting for fetching GitHub OAuth and repository settings.
- Modified extensionSettings utility functions to use arrow functions and provide a default value for getSetting.
- Improved code readability and maintainability by standardizing the method of accessing settings.

Eng 286 show when GitHub sync is disabled globally (#143)

* Refactor GitHub Sync settings in NodeConfig and GeneralSettings components

- Updated the onChange handler for GitHub Sync to use async/await and added a timeout for refreshing the config tree.
- Introduced a global settings check in NodeConfig to conditionally render the GitHub Sync checkbox and comments configuration.
- Passed setMainTab prop to NodeConfig for better navigation control.

This improves the user experience by ensuring that settings are updated correctly and provides clear feedback when global settings are disabled.

* matchingNode fix

.
mdroidian pushed a commit that referenced this pull request Jun 16, 2025
author Trang Doan <[email protected]> 1744314525 -0400
committer Michael Gartner <[email protected]> 1747354088 -0600

ENG-96 Create new relationship between nodes (#115)

* instantiate new relationship worked

* fix

* address PR comments

* fix bi-directional update issues

* show only compatible node type options

* small fix

* breakdown the components. use datacore

* working

* address PR comments

* improve search by only allowing compatible node results

* .

* rm dataview

---------

Co-authored-by: Michael Gartner <[email protected]>

Move llm-api endpoints to vercel serverless (#102)

* testing gemini

* move endgoint to website

* open ai endpoint

* added anthropic endpoint

* pass env vars

* add cors handdling and options

* .

* using centralised cors middleware

* only adding bypass cookie

* use right key

* remove the bypass token requirement

* sanitize, fix routes

* remove server action config

* DRY

* remove unused

* address review

* adress review

Roam: Add feedback toggle (#118)

* add settings to hide or show button, also works when disabled or enabled midway

* review

* .

---------

Co-authored-by: Michael Gartner <[email protected]>

[ENG-197] Fix creating link with invalid chars (#121)

* fix creating link with invalid chars

* placeholder update

---------

Co-authored-by: Michael Gartner <[email protected]>

Roam:  Add feedback button to settings menu - ENG-147 (#122)

* add button to bottom right, don't hide sdk css, tested

* remove intent not working
git

* remove ts-ignore and use a better type def

* remove styling

Update NodeConfig to use new UIDs for DiscourseNodeIndex and DiscourseNodeSpecification components (#126)

Roam: Add PostHog user identification for enhanced analytics tracking using user's roam UID as the unique identifier - ENG-177 (#123)

* add posthog identify

* remove username and email to keep it anonymus

* double userUid and best practice for js

Roam: Discourse Context Overlay - remove queue and arbitrary delay (#127)

* Refactor getOverlayInfo to use async/await and improve error handling. Update cache key from title to tag and remove overlayQueue logic for cleaner implementation.

* Remove experimental getOverlayInfo function

* Remove unused refreshUi logic

[ENG-44] Display relations (#116)

* instantiate new relationship worked

* add display relations

* remove dv

* sm fix

[ENG-198] Filtered out related file in search (#125)

* filtered out related file

* fix some naming

[ENG-97] Use TailwindCSS in obsidian app (#128)

* Update Obsidian app to integrate Tailwind CSS with PostCSS and Autoprefixer support

- Added Tailwind CSS, PostCSS, and Autoprefixer to package dependencies
- Configured styles.css to include Tailwind directives
- Enhanced compile script to process styles using PostCSS with Tailwind and Autoprefixer

* delete irrelevant package

[ENG-192] Change all existing styles to using tw (#129)

* Update Obsidian app to integrate Tailwind CSS with PostCSS and Autoprefixer support

- Added Tailwind CSS, PostCSS, and Autoprefixer to package dependencies
- Configured styles.css to include Tailwind directives
- Enhanced compile script to process styles using PostCSS with Tailwind and Autoprefixer

* delete irrelevant package

* changing all styles to tailwindcss

* Update Obsidian app to integrate Tailwind CSS with PostCSS and Autoprefixer support

- Added Tailwind CSS, PostCSS, and Autoprefixer to package dependencies
- Configured styles.css to include Tailwind directives
- Enhanced compile script to process styles using PostCSS with Tailwind and Autoprefixer

* delete irrelevant package

* changing all styles to tailwindcss

---------

Co-authored-by: Michael Gartner <[email protected]>

Roam: Bug-fix: Don't let user create discourse nodes with empty text using node context menu - ENG-171 (#130)

* functional covering all three cases tested locally

* apply coderabbit review suggestion

* better approach one that I understand and can reason about

* accidental removal of onClose

Update Roam app version to 0.13.0 in package.json and package-lock.json (#134)

[ENG-204] Move from localStorage to extensionAPI.settings (#133)

* cur progress

* address PR comments

* kinda works. need to test more

* small fix

* address PR comments

.

Create publish-obsidian.yml

Update publish-obsidian.yml

Update publish-obsidian.yml

Update publish-obsidian.yml

Enhance DiscourseContextOverlay: Update button styles to include loading state and improve score/ref display during loading. Use placeholders for score and refs when loading. (#136)

.

Update publish-obsidian.yml

Update publish-obsidian.yml

Update publish-obsidian.yml

Update publish-obsidian.yml

Refactor ExportDialog: Remove discourseGraphEnabled state and simplify FormGroup visibility logic. Set includeDiscourseContext to false by default. (#139)

Enhance LabelDialog: Add confirmText to return object for improved button text handling based on action type. (#141)

Additional styles / cursor rules (#142)

* Update STYLE_GUIDE.md and main.mdc: Add guideline for utilizing utility functions for reusable logic and common operations.

* Update STYLE_GUIDE.md and main.mdc: Add guideline to prefer arrow functions over regular function declarations.

* Update main.mdc: Add guideline to prefer Tailwind classes when refactoring inline styles.

* Update STYLE_GUIDE.md and main.mdc: Add guideline to prefer early returns over nested conditionals for improved readability.

Roam: When a user deletes a node also delete all the corresponding relations to the node - ENG-26 (#149)

* ask user for confirmation, delete corresponding relations

* address review

* address review

* address comments

[ENG-301] Create node in right-click menu (#152)

* create node in right-click menu

* small fix

* address PR comments

* address PR comments

add readme and remove sample commands

remove sample editor command

rm space

minor fixes

Roam: Bug fix - Insert Discourse Node after creation (#154)

* remove focus after menu select to allow updateBlock to work

* add clarifying comment

[ENG-308] Add command to open DG settings (#158)

* add command to open DG settings

* edit comment

ENG-322 - Switch from MIT to Apache 2.0 license (#156)

* Switch from MIT to Apache 2.0 license

* copyright discourse graphs

* rm liscense from apps/roam

---------

Co-authored-by: Michael Gartner <[email protected]>

initial port

[ENG-207] Move Github sync setting to individual nodes (#124)

* current progress

* improve in UI: if sync is turned off then also turn off the comments configuration

* address PR comments

* revert graphOverviewUid bug

* revert graphOverviewUid bug - getDiscourseNodes

* avoid racing conditions for github sync

* nested settings

* temp fix to race condition

* remove unecessary DOM and match existing styles

---------

Co-authored-by: Michael Gartner <[email protected]>

Eng 286 show when GitHub sync is disabled globally (#143)

* Refactor GitHub Sync settings in NodeConfig and GeneralSettings components

- Updated the onChange handler for GitHub Sync to use async/await and added a timeout for refreshing the config tree.
- Introduced a global settings check in NodeConfig to conditionally render the GitHub Sync checkbox and comments configuration.
- Passed setMainTab prop to NodeConfig for better navigation control.

This improves the user experience by ensuring that settings are updated correctly and provides clear feedback when global settings are disabled.

* matchingNode fix

.

Refactor Export components to use getSetting for consistent settings retrieval

- Updated ExportDialog and ExportGithub components to replace localStorageGet with getSetting for fetching GitHub OAuth and repository settings.
- Modified extensionSettings utility functions to use arrow functions and provide a default value for getSetting.
- Improved code readability and maintainability by standardizing the method of accessing settings.

Eng 286 show when GitHub sync is disabled globally (#143)

* Refactor GitHub Sync settings in NodeConfig and GeneralSettings components

- Updated the onChange handler for GitHub Sync to use async/await and added a timeout for refreshing the config tree.
- Introduced a global settings check in NodeConfig to conditionally render the GitHub Sync checkbox and comments configuration.
- Passed setMainTab prop to NodeConfig for better navigation control.

This improves the user experience by ensuring that settings are updated correctly and provides clear feedback when global settings are disabled.

* matchingNode fix

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

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants