Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feast(backend): ux datamap doc #49

Merged
merged 3 commits into from
Nov 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions backend/src/build-system/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ import {
} from './types';
import { Logger } from '@nestjs/common';

export type GlobalDataKeys = 'projectName' | 'description' | 'platform';
type ContextData = {
[key in GlobalDataKeys]: string;
} & Record<string, any>;

export class BuilderContext {
private state: BuildExecutionState = {
completed: new Set(),
Expand Down Expand Up @@ -80,11 +85,17 @@ export class BuilderContext {
return { ...this.state };
}

setData(key: string, value: any): void {
setData<Key extends keyof ContextData>(
key: Key,
value: ContextData[Key],
): void {
this.data[key] = value;
}

getData(key: string): any {
getData<Key extends keyof ContextData>(key: Key): ContextData[Key] {
if (!(key in this.data)) {
throw new Error(`Data key "${key}" is not set or does not exist.`);
}
return this.data[key];
}

Expand Down
40 changes: 40 additions & 0 deletions backend/src/build-system/node/ux-datamap/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import { BuildHandler, BuildResult } from 'src/build-system/types';
import { BuilderContext } from 'src/build-system/context';
import { ModelProvider } from 'src/common/model-provider';
import { prompts } from './prompt';

export class UXDatamapHandler implements BuildHandler {
readonly id = 'op:UX_DATAMAP::STATE:GENERATE';

async run(context: BuilderContext, args: unknown): Promise<BuildResult> {
console.log('Generating UX Data Map Document...');
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Replace console.log with structured logging.

Consider using the BuilderContext's logger for consistent logging across the build system.

-    console.log('Generating UX Data Map Document...');
+    context.logger.info('Generating UX Data Map Document...');

Committable suggestion skipped: line range outside the PR's diff.


// extract relevant data from the context
const projectName =
context.getData('projectName') || 'Default Project Name';
const uxGoals = context.getData('uxGoals') || 'Default UX Goals';

// generate the UX Data Map prompt dynamically
const prompt = prompts.generateUXDataMapPrompt(
projectName,
args as string,
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add type validation for args parameter.

The unsafe type casting of args as string could lead to runtime errors. Consider adding type validation before casting.

-      args as string,
+      typeof args === 'string' ? args : String(args),
📝 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.

Suggested change
args as string,
typeof args === 'string' ? args : String(args),

// TODO: change later
'web',
);
Comment on lines +21 to +23
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Remove hardcoded platform value.

The hardcoded 'web' value with a TODO comment should be replaced with a configurable value, possibly from the context or args.

-      // TODO: change later
-      'web',
+      context.getData('platform') || 'web',
📝 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.

Suggested change
// TODO: change later
'web',
);
context.getData('platform') || 'web',
);


// Use ModelProsvider or another service to generate the document
const uxDatamapContent = await context.model.chatSync(
{
content: prompt,
},
'gpt-3.5-turbo',
);
Comment on lines +25 to +31
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add error handling for model generation.

The model interaction lacks error handling and might fail silently. Consider wrapping it in a try-catch block and handling potential errors.

-    const uxDatamapContent = await context.model.chatSync(
-      {
-        content: prompt,
-      },
-      'gpt-3.5-turbo',
-    );
+    try {
+      const uxDatamapContent = await context.model.chatSync(
+        {
+          content: prompt,
+        },
+        'gpt-3.5-turbo',
+      );
+      if (!uxDatamapContent) {
+        throw new Error('Empty response from model');
+      }
+    } catch (error) {
+      context.logger.error('Failed to generate UX Data Map', { error });
+      return {
+        success: false,
+        error: `Failed to generate UX Data Map: ${error.message}`,
+      };
+    }
📝 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.

Suggested change
// Use ModelProsvider or another service to generate the document
const uxDatamapContent = await context.model.chatSync(
{
content: prompt,
},
'gpt-3.5-turbo',
);
// Use ModelProsvider or another service to generate the document
try {
const uxDatamapContent = await context.model.chatSync(
{
content: prompt,
},
'gpt-3.5-turbo',
);
if (!uxDatamapContent) {
throw new Error('Empty response from model');
}
} catch (error) {
context.logger.error('Failed to generate UX Data Map', { error });
return {
success: false,
error: `Failed to generate UX Data Map: ${error.message}`,
};
}

Comment on lines +26 to +31
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Consider centralizing the model name configuration

The model name 'gpt-3.5-turbo' is used in multiple places with different approaches:

  • In llm-server/src/main.ts it's used as a default value
  • In backend/src/build-system/node/ux-datamap/index.ts and product_requirements_document/prd.ts it's hardcoded
  • The type definition in llm-server/src/type/GenerateMessage.ts shows it's a configurable parameter

Consider:

  • Moving the model name to a centralized configuration
  • Using the model parameter from the ModelProvider consistently instead of hardcoding
🔗 Analysis chain

Verify model name consistency across the codebase.

The hardcoded model name 'gpt-3.5-turbo' should be consistent with other parts of the codebase.


Let me analyze the actual usage context of the model name to ensure consistency and determine if there's a need for centralized configuration.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for other occurrences of model names to ensure consistency
rg -l "gpt-3.5-turbo"

# Search for potential model name constants or configurations
rg -l "MODEL_NAME|ModelName|model.*name" --type ts

Length of output: 297


Script:

#!/bin/bash
# Check the content of files using gpt-3.5-turbo to understand the context
rg "gpt-3.5-turbo" -C 3

# Check if there are any model configuration files
fd -e ts -e js config

Length of output: 2224


// Store the generated document in the context
context.setData('uxDatamapDocument', uxDatamapContent);
return {
success: true,
data: uxDatamapContent,
};
}
}
140 changes: 140 additions & 0 deletions backend/src/build-system/node/ux-datamap/prompt.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
export const prompts = {
generateUXDataMapPrompt: (
projectName: string,
sitemapDoc: string,
platform: string,
): string => {
Comment on lines +1 to +6
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add input validation for required parameters.

The function parameters should be validated to ensure they contain meaningful values.

 export const prompts = {
   generateUXDataMapPrompt: (
     projectName: string,
     sitemapDoc: string,
     platform: string,
   ): string => {
+    if (!projectName?.trim()) throw new Error('Project name is required');
+    if (!sitemapDoc?.trim()) throw new Error('Sitemap documentation is required');
+    if (!platform?.trim()) throw new Error('Platform is required');
📝 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.

Suggested change
export const prompts = {
generateUXDataMapPrompt: (
projectName: string,
sitemapDoc: string,
platform: string,
): string => {
export const prompts = {
generateUXDataMapPrompt: (
projectName: string,
sitemapDoc: string,
platform: string,
): string => {
if (!projectName?.trim()) throw new Error('Project name is required');
if (!sitemapDoc?.trim()) throw new Error('Sitemap documentation is required');
if (!platform?.trim()) throw new Error('Platform is required');

return `You are an expert UX Designer. Your task is to analyze the provided sitemap documentation and identify all the data elements needed to support the user experience, based on the following inputs:

- Project name: ${projectName}
- Sitemap Documentation: ${sitemapDoc}
- Platform: ${platform}

Follow these guidelines to analyze data requirements from a UX perspective:

### Instructions and Rules:

1. For each page/screen in the sitemap:
- What information does the user need to see?
- What data does the user need to input?
- What feedback or responses should the user receive?
- What dynamic content needs to be displayed?

2. Consider:
- User goals on each page
- Required form fields and their purposes
- Content that needs to be displayed
- System feedback and status information
- Dynamic updates and real-time data
- Error states and messages

### UX Data Requirements Document:

---
### Page-by-Page Data Analysis

#### 1. Project Overview
- **Project Name**:
- **Platform**:
- **General Description**:

#### 2. Global Data Elements
Common data elements needed across multiple pages:
- User profile information
- Navigation states
- System status
- etc.

#### 3. Page-Specific Data Requirements

For each page in sitemap:

##### [Page Name]
**Purpose**: [Brief description of page purpose]

**User Goals**:
- Goal 1
- Goal 2
...

**Required Data Elements**:

*Input Data*:
- Field 1: [Purpose and importance]
- Why it's needed
- User expectations
- Requirements (if any)
- Field 2: ...

*Display Data*:
- Content 1: [Purpose and importance]
- Why users need this information
- Update frequency (if dynamic)
- Priority level
- Content 2: ...

*Feedback & States*:
- Success states
- Error states
- Loading states
- Empty states

**User Interactions**:
- How data changes based on user actions
- What feedback is needed
- When/how data updates

Example for Login Page:

##### Login Page
**Purpose**: Allow users to authenticate and access their account

**User Goals**:
- Sign into their account
- Recover forgotten password
- Stay signed in for convenience

**Required Data Elements**:

*Input Data*:
- Username/Email
- Purpose: Identify the user account
- User expectation: Email format or username rules
- Should be remembered if user chooses
- Password
- Purpose: Authenticate the user
- Should be masked for security
- Should support paste functionality
- "Remember Me" option
- Purpose: Convenience for returning users
- Optional selection

*Display Data*:
- Login form status
- Authentication feedback
- Password requirements (if needed)
- Account recovery options

*Feedback & States*:
- Loading state during authentication
- Success feedback and redirect
- Error messages for invalid credentials
- Password recovery confirmation

**User Interactions**:
- Form validation feedback
- Login button state changes
- Immediate feedback on input errors
- Clear path to password recovery

---

Your reply must start with: "\`\`\`UXDataMap" and end with "\`\`\`".

Focus on describing the data requirements from a user experience perspective. For each page:
1. What data needs to be collected and why
2. What information needs to be displayed and why
3. How the data supports user goals
4. What feedback the user needs`;
},
};
Loading