Conversation
…s and Vite aliasing to use local mocks\n- Add Vitest + @storybook/test + testing-library + happy-dom\n- Add vitest config and a sample story-based test
…uButton, Signin, Chat, SigninButton)
WalkthroughAdds Storybook configuration, Vue component stories, testing infrastructure (Vitest), mock utilities for Firebase/social auth, GitHub Actions workflow for Firebase preview deployments, repository guidelines, and substantial package.json dependency additions for GraphAI agents, Storybook, and testing tools. Changes
Sequence DiagramsequenceDiagram
actor User
participant GitHub
participant Actions as GitHub Actions
participant Firebase as Firebase Hosting
participant PR as PR Comments
User->>GitHub: Open/sync PR
GitHub->>Actions: Trigger deploy-preview workflow
rect rgb(200, 220, 240)
Note over Actions: Skip if 'no-preview' label
Actions->>Actions: Checkout code
Actions->>Actions: Setup Node.js 20
Actions->>Actions: Install dependencies
Actions->>Actions: Create symlink (config)
Actions->>Actions: Build project (Vite)
end
Actions->>Firebase: Deploy to preview channel<br/>(pr-<PR number>)
Firebase-->>Actions: Deployment success
Actions->>PR: Post/update comment<br/>with preview URL
PR-->>User: Preview URL visible
Note over Firebase: Preview expires in 7 days
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Areas requiring extra attention:
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (13)
packages/grapys-vue/.storybook/tsconfig.json (1)
14-14: Consider adding common exclusions for TypeScript compilation.The empty
excludearray may result in TypeScript compiling unnecessary files likenode_modules,dist, orbuilddirectories, which could slow down type checking and compilation.Apply this diff to exclude common directories:
- "exclude": [] + "exclude": [ + "../node_modules", + "../dist", + "../build" + ]packages/grapys-vue/src/storybook/mocks/firebase.ts (1)
22-24: Empty object placeholders may cause runtime errors.The
firebaseApp,db, andfunctionsexports are typed asRecord<string, never>, meaning they're empty objects. If Storybook stories or tests attempt to access properties or methods on these objects, they'll receiveundefinedand may throw runtime errors.Consider providing Proxy-based mocks that log warnings for unimplemented access:
-export const firebaseApp = {} as Record<string, never>; -export const db = {} as Record<string, never>; -export const functions = {} as Record<string, never>; +export const firebaseApp = new Proxy({}, { + get(_, prop) { + console.warn(`[Mock] firebaseApp.${String(prop)} accessed but not implemented`); + return undefined; + } +}) as any; +export const db = new Proxy({}, { + get(_, prop) { + console.warn(`[Mock] db.${String(prop)} accessed but not implemented`); + return undefined; + } +}) as any; +export const functions = new Proxy({}, { + get(_, prop) { + console.warn(`[Mock] functions.${String(prop)} accessed but not implemented`); + return undefined; + } +}) as any;packages/grapys-vue/src/components/SideMenuButton.stories.ts (1)
35-55: Consider simplifying story definitions.The Primary and Danger stories use identical render patterns. Since they only differ in args and slot content, you could omit the explicit
renderfunctions and let Storybook use the default renderer with slot text passed via args.For example, you could use Storybook's slot handling:
export const Primary: Story = { args: { variant: "primary", + default: "Primary Action", }, - render: (args) => ({ - components: { SideMenuButton }, - setup: () => ({ args }), - template: '<SideMenuButton v-bind="args">Primary Action</SideMenuButton>', - }), };However, the current explicit approach is also clear and works correctly.
packages/grapys-vue/src/__tests__/storybook.spec.ts (1)
1-15: LGTM: Good example of Storybook + Vitest integration.This test demonstrates the pattern for testing composed Storybook stories:
composeStoriesintegrates story metadata with the component- Testing Library provides DOM queries
- Basic assertion confirms rendering
This pattern can be extended to test other stories and interactions.
Consider expanding test coverage to verify:
- Different story variants (not just Default)
- Component interactions and state changes
- Accessibility properties
.github/workflows/deploy-preview.yml (1)
29-31: Consider build-time environment configuration as an alternative.The symlink approach works well for switching configurations. As an alternative, you could use environment variables or build-time config injection to avoid file system manipulation in CI.
packages/grapys-vue/src/components/Node2.stories.ts (3)
29-30: Consider using structuredClone for safer deep cloning.The JSON.parse/stringify approach works but has limitations (loses functions, Date objects, undefined values, etc.). Modern environments support
structuredClonewhich handles more data types correctly.Apply this diff:
-const cloneNodeData = (node: GUINodeData): GUINodeData => - JSON.parse(JSON.stringify(node)) as GUINodeData; +const cloneNodeData = (node: GUINodeData): GUINodeData => + structuredClone(node);
77-77: Simplify property existence check.The verbose
Object.prototype.hasOwnProperty.callcan be replaced with theinoperator for better readability.Apply this diff:
- if (Object.prototype.hasOwnProperty.call(storyArgs, "resultUrl")) { + if ("resultUrl" in storyArgs) {
22-22: Avoid type assertions withas any.Type assertions to
anybypass TypeScript's type checking. Consider defining a proper interface for argTypes or using a more specific type.packages/grapys-vue/src/components/Node.stories.ts (3)
29-30: Consider using structuredClone for safer deep cloning.Same issue as in Node2.stories.ts - the JSON.parse/stringify approach has limitations with certain data types.
77-77: Simplify property existence check.Same verbose syntax as in Node2.stories.ts - use the
inoperator instead.
22-22: Avoid type assertions withas any.Same issue as Node2.stories.ts - define proper types for argTypes.
packages/grapys-vue/src/components/Layout.stories.ts (2)
11-28: Consider improving type safety for custom story args.The
as anytype assertions on lines 24 and 27 bypass TypeScript's type checking for the customauthStatecontrol. While this pattern is common in Storybook for story-specific controls that aren't component props, it reduces type safety.Consider defining an explicit type for the story args:
+type LayoutStoryArgs = { + authState: "loading" | "signedOut" | "signedIn"; +}; + const meta: Meta<typeof Layout> = { title: "Components/Layout", component: Layout, tags: ["autodocs"], parameters: { layout: "fullscreen", }, argTypes: { authState: { control: "select", options: ["loading", "signedOut", "signedIn"], description: "Mocked Firebase authentication state for the story.", }, - } as any, + } satisfies Record<keyof LayoutStoryArgs, any>, args: { authState: "loading", - } as any, + } satisfies LayoutStoryArgs, };
61-61: Silent error handling may hide routing issues.The catch handler silently ignores all routing errors. While this may be intentional for the Storybook environment, it could mask real problems during story development.
Consider logging the error for debugging:
- router.push("/").catch(() => undefined); + router.push("/").catch((err) => console.warn("Story router navigation failed:", err));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (27)
.github/workflows/deploy-preview.yml(1 hunks).gitignore(0 hunks)AGENTS.md(1 hunks)docs/firebase-staging-deploy.md(1 hunks)packages/grapys-vue/.storybook/main.ts(1 hunks)packages/grapys-vue/.storybook/preview.ts(1 hunks)packages/grapys-vue/.storybook/tsconfig.json(1 hunks)packages/grapys-vue/package.json(1 hunks)packages/grapys-vue/src/.storybook/mocks/SocialLogin.ts(1 hunks)packages/grapys-vue/src/.storybook/mocks/firebase.ts(1 hunks)packages/grapys-vue/src/__tests__/storybook.spec.ts(1 hunks)packages/grapys-vue/src/agents/event.ts(1 hunks)packages/grapys-vue/src/components/Blank.stories.ts(1 hunks)packages/grapys-vue/src/components/Chat.stories.ts(1 hunks)packages/grapys-vue/src/components/Layout.stories.ts(1 hunks)packages/grapys-vue/src/components/Node.stories.ts(1 hunks)packages/grapys-vue/src/components/Node.ts(1 hunks)packages/grapys-vue/src/components/Node2.stories.ts(1 hunks)packages/grapys-vue/src/components/Node2.ts(1 hunks)packages/grapys-vue/src/components/SideMenuButton.stories.ts(1 hunks)packages/grapys-vue/src/components/Signin.stories.ts(1 hunks)packages/grapys-vue/src/components/SigninButton.stories.ts(1 hunks)packages/grapys-vue/src/config/project.ts(1 hunks)packages/grapys-vue/src/storybook/mocks/SocialLogin.ts(1 hunks)packages/grapys-vue/src/storybook/mocks/firebase.ts(1 hunks)packages/grapys-vue/src/storybook/useStoryApp.ts(1 hunks)packages/grapys-vue/vitest.config.ts(1 hunks)
💤 Files with no reviewable changes (1)
- .gitignore
🧰 Additional context used
🧬 Code graph analysis (6)
packages/grapys-vue/src/components/Signin.stories.ts (1)
packages/grapys-vue/src/components/SigninButton.stories.ts (1)
Default(17-22)
packages/grapys-vue/src/components/Layout.stories.ts (2)
packages/grapys-vue/src/storybook/useStoryApp.ts (1)
useStoryApp(4-12)packages/grapys-vue/src/store/firebase.ts (1)
useFirebaseStore(5-24)
packages/grapys-vue/src/components/SigninButton.stories.ts (1)
packages/grapys-vue/src/components/Signin.stories.ts (1)
Default(18-23)
packages/grapys-vue/src/components/Node2.stories.ts (2)
packages/grapys-vue/src/storybook/useStoryApp.ts (1)
useStoryApp(4-12)packages/grapys-vue/src/store/index.ts (1)
useStore(19-267)
packages/grapys-vue/src/components/Node.stories.ts (2)
packages/grapys-vue/src/storybook/useStoryApp.ts (1)
useStoryApp(4-12)packages/grapys-vue/src/store/index.ts (1)
useStore(19-267)
packages/grapys-vue/src/components/Blank.stories.ts (1)
packages/grapys-vue/src/storybook/useStoryApp.ts (1)
useStoryApp(4-12)
🔇 Additional comments (26)
packages/grapys-vue/src/agents/event.ts (1)
4-4: Type definition looks reasonable but verify runtime contract.The
EventWithRejecttype definition extendingEventDatawith arejectmethod is structurally sound and follows Promise-like semantics.However, ensure this aligns with the actual data provided by
@receptron/event_agent_generator.packages/grapys-vue/src/components/Node.ts (1)
1-1: LGTM! Clean barrel export pattern.The re-export provides a clean interface for consumers to import the Node component, aligning with the similar pattern used in Node2.ts.
AGENTS.md (1)
1-41: LGTM! Comprehensive repository guidelines.The documentation provides clear guidelines for the monorepo setup, covering project structure, development workflows, coding conventions, and security considerations. This is a valuable addition for team collaboration.
packages/grapys-vue/src/components/Signin.stories.ts (1)
1-24: LGTM! Standard Storybook configuration.The story follows Storybook best practices for Vue 3, with proper type definitions, autodocs tag, and action handling configuration.
packages/grapys-vue/src/components/Blank.stories.ts (1)
40-40: Verify silent error handling is intentional.The router navigation error is silently ignored with
.catch(() => undefined). While this might be intentional for Storybook stories to prevent console noise, ensure this doesn't hide legitimate routing issues during story development.docs/firebase-staging-deploy.md (1)
1-57: LGTM! Comprehensive Firebase preview deployment documentation.The documentation clearly explains the preview deployment workflow, setup requirements, and operational procedures. This will be valuable for maintainers configuring the Firebase hosting integration.
packages/grapys-vue/src/storybook/mocks/firebase.ts (2)
7-14: LGTM! Solid auth state listener implementation.The listener management using a Set provides proper subscription/unsubscription behavior and the public
triggerAuthStateChangefunction enables effective testing of auth state changes.
1-24: Duplicate Firebase mock files do not exist.The script output confirms there is only a single Firebase mock file at
packages/grapys-vue/src/storybook/mocks/firebase.ts. The claimed duplicate at.storybook/mocks/firebase.tsdoes not exist. All references in bothvitest.config.tsand.storybook/main.tsconsistently point to the samesrc/storybook/mocks/location. No maintenance issues or inconsistencies stem from duplicate mocks.Likely an incorrect or invalid review comment.
packages/grapys-vue/src/components/SideMenuButton.stories.ts (2)
5-29: LGTM: Well-structured meta configuration.The Storybook meta configuration is properly typed and follows Vue 3 conventions. The argTypes and default args provide a good starting point for interactive documentation.
57-66: LGTM: Disabled story properly demonstrates the disabled state.The story correctly sets the
disabledprop and provides appropriate slot content.packages/grapys-vue/package.json (4)
8-8: Verify that Storybook should be built in every build.The build script now includes
yarn storybook:build, which couples Storybook build to the main build process. This could increase build times.Confirm this is intentional. If Storybook is only needed for development or specific deployments, consider keeping it as a separate script.
12-15: LGTM: Testing and Storybook scripts are well-structured.The scripts follow best practices:
- Vitest configuration is explicit
- Storybook test script properly uses
start-server-and-testto ensure the server is running before tests- Output directory for Storybook build is clearly specified
38-52: LGTM: Appropriate devDependencies for Storybook and testing setup.The devDependencies include:
- Complete Storybook toolchain (core, addons, test-runner)
- Vitest with Vue testing library and happy-dom
- Build/style tooling (Vite, Tailwind, PostCSS)
All dependencies align with the PR's goal of adding Storybook for UI development.
33-33: React dependency is correctly included and intentionally required.Verification confirms React is used in
packages/grapys-vue/src/agents/event_react.ts, which imports React hooks (useState,useMemo). This is a GraphAI agent integration that requires React, making the dependency legitimate for this Vue project.packages/grapys-vue/src/components/Node2.ts (1)
1-2: LGTM: Standard barrel export pattern.The re-export from
../views/Node2.vuefollows the existing pattern used inNode.tsand provides a clean public API for the component.packages/grapys-vue/src/storybook/useStoryApp.ts (1)
4-12: LGTM: Well-implemented composition helper.The
useStoryApphook correctly:
- Uses
getCurrentInstance()to access the Vue app instance- Validates it's called within a proper context
- Provides a clear error message
- Returns the app instance for use in Storybook stories
This is a clean utility for accessing the app/Pinia stores within story setup contexts.
packages/grapys-vue/src/storybook/mocks/SocialLogin.ts (1)
1-10: Consider whether errors should propagate after errorCallback.The mock function catches errors from the callback and forwards them to
errorCallback, but doesn't re-throw. This means:
- Errors are silently swallowed after being logged to
errorCallback- The returned function always succeeds even if callback throws
Verify whether this behavior matches the real
googleSignin:
- Should errors propagate to the caller after
errorCallbackhandles them?- Or should
errorCallbackfully handle the error?If errors should propagate:
} catch (error) { if (error instanceof Error) { errorCallback?.(error); + throw error; } }Otherwise, the current implementation is acceptable for Storybook mocks.
packages/grapys-vue/.storybook/preview.ts (2)
7-11: LGTM: Proper Pinia setup for Storybook.The setup function correctly:
- Creates a Pinia instance
- Sets it as active with
setActivePinia(pinia)- Registers it with the Vue app
This ensures all stories have access to Pinia stores.
13-23: LGTM: Standard Storybook preview parameters.The preview configuration includes sensible defaults:
- Actions auto-detection for event handlers (props starting with "on")
- Control matchers for common types (color, date)
These provide good out-of-the-box behavior for interactive documentation.
packages/grapys-vue/src/components/SigninButton.stories.ts (1)
1-22: LGTM!The Storybook story configuration is well-structured and follows Vue 3 best practices with proper TypeScript typing.
packages/grapys-vue/.storybook/main.ts (1)
1-42: LGTM!The Storybook configuration properly integrates Vue 3, Vite, and TailwindCSS with well-structured path aliases for mocking Firebase and social login utilities in the story environment.
packages/grapys-vue/vitest.config.ts (1)
1-33: LGTM!The Vitest configuration is properly set up with happy-dom for Vue component testing and maintains consistent path aliases with the Storybook configuration.
packages/grapys-vue/src/components/Chat.stories.ts (1)
1-56: LGTM!The Chat stories effectively demonstrate different chat states including streaming messages, with realistic sample data that showcases user, assistant, and error message types.
.github/workflows/deploy-preview.yml (1)
1-48: LGTM!The workflow properly configures Firebase preview deployments with appropriate permissions, PR-based channel naming, and resource management via 7-day expiry.
packages/grapys-vue/src/.storybook/mocks/firebase.ts (1)
1-25: LGTM!The Firebase auth mock is clean and effective, properly implementing the listener pattern with an unsubscribe mechanism and providing a test helper for triggering auth state changes.
packages/grapys-vue/src/components/Layout.stories.ts (1)
66-79: No changes needed to the mock user properties.The mock user with only
displayNameis sufficient for the Layout story. Layout.vue only accessesfirebaseStore.isSignedIn(a computed Boolean), not individual User properties. Signin.vue (the only component referenced in Layout) also doesn't access User properties.While
uidis accessed elsewhere in the codebase (SideMenuSaveFirebase.vue), that component is not part of Layout's tested hierarchy in this story.
| export const googleSignin = (callback?: () => void, errorCallback?: (error: Error) => void) => () => { | ||
| try { | ||
| callback?.(); | ||
| console.info("[Mock] googleSignin invoked"); | ||
| } catch (error) { | ||
| if (error instanceof Error) { | ||
| errorCallback?.(error); | ||
| } | ||
| } | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the need for duplicate mock files.
According to the AI summary, a duplicate mock file exists at packages/grapys-vue/src/storybook/mocks/SocialLogin.ts. Consider consolidating these mocks to avoid maintenance overhead and potential inconsistencies.
Run the following script to verify the duplicate mocks:
🏁 Script executed:
#!/bin/bash
# Description: Check for duplicate SocialLogin mock files and compare their contents
fd -t f "SocialLogin.ts" packages/grapys-vue/src
echo "=== Comparing mock files ==="
diff -u packages/grapys-vue/src/.storybook/mocks/SocialLogin.ts packages/grapys-vue/src/storybook/mocks/SocialLogin.ts || echo "Files differ or one doesn't exist"Length of output: 137
I need to verify the exact file locations and contents. The diff produced no output, which suggests the .storybook file may not exist. Let me gather more details:
---
🏁 Script executed:
```shell
#!/bin/bash
# Description: Verify file existence and compare mock file contents
# Check if .storybook/mocks/SocialLogin.ts exists
if [ -f "packages/grapys-vue/src/.storybook/mocks/SocialLogin.ts" ]; then
echo "✓ File exists: packages/grapys-vue/src/.storybook/mocks/SocialLogin.ts"
else
echo "✗ File does NOT exist: packages/grapys-vue/src/.storybook/mocks/SocialLogin.ts"
fi
# Check if storybook/mocks/SocialLogin.ts exists
if [ -f "packages/grapys-vue/src/storybook/mocks/SocialLogin.ts" ]; then
echo "✓ File exists: packages/grapys-vue/src/storybook/mocks/SocialLogin.ts"
echo ""
echo "=== Content of packages/grapys-vue/src/storybook/mocks/SocialLogin.ts ==="
cat "packages/grapys-vue/src/storybook/mocks/SocialLogin.ts"
else
echo "✗ File does NOT exist: packages/grapys-vue/src/storybook/mocks/SocialLogin.ts"
fi
# List all SocialLogin files found
echo ""
echo "=== All SocialLogin files in packages/grapys-vue/src ==="
fd -t f "SocialLogin.ts" packages/grapys-vue/src
Length of output: 658
---
🏁 Script executed:
```shell
#!/bin/bash
# Description: Compare the two mock files and show utils version
echo "=== Comparing .storybook/mocks vs storybook/mocks ==="
diff -u packages/grapys-vue/src/.storybook/mocks/SocialLogin.ts packages/grapys-vue/src/storybook/mocks/SocialLogin.ts || true
echo ""
echo "=== Content of packages/grapys-vue/src/.storybook/mocks/SocialLogin.ts ==="
cat packages/grapys-vue/src/.storybook/mocks/SocialLogin.ts
echo ""
echo "=== Content of packages/grapys-vue/src/utils/firebase/SocialLogin.ts ==="
cat packages/grapys-vue/src/utils/firebase/SocialLogin.ts
Length of output: 1181
Consolidate duplicate mock files in .storybook and storybook directories.
Verification confirms two identical SocialLogin.ts mock files exist at packages/grapys-vue/src/.storybook/mocks/SocialLogin.ts and packages/grapys-vue/src/storybook/mocks/SocialLogin.ts. Having duplicate mocks introduces maintenance overhead and risks inconsistencies if one is updated without the other. Consolidate to a single location and update all references accordingly.
🤖 Prompt for AI Agents
In packages/grapys-vue/src/.storybook/mocks/SocialLogin.ts lines 1-10: there are
duplicate mock files at packages/grapys-vue/src/.storybook/mocks/SocialLogin.ts
and packages/grapys-vue/src/storybook/mocks/SocialLogin.ts; consolidate by
choosing a single canonical location (prefer the .storybook path), remove the
other file, update all imports/usages across the repo to reference the chosen
path, adjust any barrel/index or tsconfig path mappings if used, and run a
global search (e.g., ripgrep/IDE) plus the build/tests to ensure no broken
imports remain.
| } catch (error) { | ||
| if (error instanceof Error) { | ||
| errorCallback?.(error); | ||
| } | ||
| } |
There was a problem hiding this comment.
Incomplete error handling for non-Error throw values.
The catch block only handles Error instances. If the callback throws a non-Error value (string, number, object, etc.), it won't be passed to errorCallback.
Apply this diff to handle all thrown values:
} catch (error) {
- if (error instanceof Error) {
- errorCallback?.(error);
- }
+ errorCallback?.(error instanceof Error ? error : new Error(String(error)));
}📝 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.
| } catch (error) { | |
| if (error instanceof Error) { | |
| errorCallback?.(error); | |
| } | |
| } | |
| } catch (error) { | |
| errorCallback?.(error instanceof Error ? error : new Error(String(error))); | |
| } |
🤖 Prompt for AI Agents
In packages/grapys-vue/src/.storybook/mocks/SocialLogin.ts around lines 5 to 9,
the catch only forwards values that are instanceof Error; modify it to pass all
thrown values to errorCallback by calling errorCallback with the original error
when it is an Error and otherwise wrap the thrown non-Error value into a new
Error (e.g., new Error(String(error))) before passing it so
strings/numbers/objects are not dropped and the callback always receives an
Error object.
| const eventsObj = ref<Record<string, EventWithReject>>({}); | ||
| const { eventAgent } = eventAgentGenerator((id, data) => { | ||
| eventsObj.value[id] = data; | ||
| eventsObj.value[id] = data as EventWithReject; | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
Unsafe type assertion may cause runtime errors.
The cast data as EventWithReject on line 11 asserts that the data parameter includes a reject method without verification. If @receptron/event_agent_generator provides EventData without this method, calling event.reject() in clearEvents (line 28) will throw a runtime error.
Verify that the library actually provides the reject method by checking the library's type definitions:
Consider adding a type guard or runtime check:
const { eventAgent } = eventAgentGenerator((id, data) => {
- eventsObj.value[id] = data as EventWithReject;
+ if ('reject' in data && typeof data.reject === 'function') {
+ eventsObj.value[id] = data as EventWithReject;
+ } else {
+ console.error('EventData missing reject method:', data);
+ }
});Or use optional chaining in clearEvents:
const clearEvents = () => {
- Object.values(eventsObj.value).forEach((event) => event.reject());
+ Object.values(eventsObj.value).forEach((event) => event.reject?.());
eventsObj.value = {};
};🏁 Script executed:
#!/bin/bash
# Description: Verify EventData type definition from @receptron/event_agent_generator
# Check if reject method is part of EventData or needs to be added
# Find and examine the library's type definitions
fd -e ts -e d.ts . node_modules/@receptron/event_agent_generator --exec cat {} \; | rg -A 10 "interface EventData|type EventData"
# Also search for reject method in the library
rg -n "reject" node_modules/@receptron/event_agent_generatorLength of output: 218
I'll verify the library contract using a different approach. Let me check the package.json for the library version and search for its documentation.The web search didn't return results for the specific @receptron/event_agent_generator npm package. Let me search more directly for this package on npm.
Let me check the repository's package.json and examine how the library is used more directly.
Also, let me search the codebase to understand the actual contract:
🏁 Script executed:
#!/bin/bash
# Find package.json and check dependencies for @receptron/event_agent_generator
find . -name "package.json" -type f | head -5 | xargs -I {} sh -c 'echo "=== {} ===" && cat {}'Length of output: 6225
🏁 Script executed:
#!/bin/bash
# Search for EventWithReject type definition and how it's used
rg -n "EventWithReject|EventData" --type ts -C 5
# Also check imports from the library
rg -n "@receptron/event_agent_generator" --type tsLength of output: 15892
Based on my analysis of the codebase, I can confirm the review comment identifies a genuine and critical issue.
Key findings:
The type definition on line 4 of event.ts reveals the problem:
type EventWithReject = EventData & { reject: (reason?: any) => void };This type intersection proves that EventData from @receptron/event_agent_generator does NOT include a reject method. The developer is explicitly adding it via intersection. The unsafe cast on line 11 (data as EventWithReject) bypasses TypeScript's type checking, masking a contract violation.
If clearEvents (referenced in the review comment at line 28) calls event.reject() without first verifying the method exists, this will throw a runtime error.
Conclusion: The original review comment is valid and warranted.
Rewritten review comment:
Fix unsafe type assertion that bypasses type safety.
Line 11 casts data to EventWithReject without runtime verification, but EventData from @receptron/event_agent_generator v0.0.6 does not include a reject method (see type definition on line 4). When clearEvents calls event.reject(), this will fail at runtime if reject is not actually present.
- Add defensive runtime check before storing:
const { eventAgent } = eventAgentGenerator((id, data) => {
- eventsObj.value[id] = data as EventWithReject;
+ if ('reject' in data && typeof data.reject === 'function') {
+ eventsObj.value[id] = data as EventWithReject;
+ } else {
+ console.warn('EventData missing reject method', data);
+ }
});Or use optional chaining in clearEvents as defensive measure:
Object.values(eventsObj.value).forEach((event) => event.reject?.());🤖 Prompt for AI Agents
In packages/grapys-vue/src/agents/event.ts around lines 9 to 12, the code
unsafely casts incoming event data to EventWithReject even though EventData from
@receptron/event_agent_generator does not include reject; remove the "as
EventWithReject" assertion and instead store the value as its original EventData
type, and add a defensive runtime check so you only store/mark an event as
having a reject method if typeof data.reject === 'function' (or alternatively
keep stored events as EventData and ensure clearEvents uses optional chaining or
checks before calling event.reject()); this eliminates the unsafe cast and
prevents runtime failures when reject is absent.
| import type { Meta, StoryObj } from "@storybook/vue3"; | ||
| import { action } from "@storybook/addon-actions"; | ||
| import { computed, toRef, watch } from "vue"; | ||
| import { createPinia, setActivePinia } from "pinia"; | ||
|
|
||
| import Node from "./Node"; | ||
| import { useStore } from "../store"; | ||
| import type { GUINodeData, UpdateNodePositionData, UpdateStaticValue } from "../utils/gui/type"; | ||
| import { useStoryApp } from "../storybook/useStoryApp"; | ||
|
|
||
| const meta: Meta<typeof Node> = { | ||
| title: "Components/Node", | ||
| component: Node, | ||
| tags: ["autodocs"], | ||
| argTypes: { | ||
| nodeData: { control: false }, | ||
| nearestData: { control: false }, | ||
| resultUrl: { | ||
| control: "text", | ||
| description: "Optional image URL to showcase Node result rendering.", | ||
| }, | ||
| } as any, | ||
| }; | ||
|
|
||
| export default meta; | ||
|
|
||
| type Story = StoryObj<typeof meta>; | ||
|
|
||
| const cloneNodeData = (node: GUINodeData): GUINodeData => | ||
| JSON.parse(JSON.stringify(node)) as GUINodeData; | ||
|
|
||
| const createRender = (): Story["render"] => | ||
| (args) => { | ||
| const storyArgs = args as { | ||
| nodeData: GUINodeData; | ||
| nodeIndex: number; | ||
| isConnectable: boolean; | ||
| nearestData?: unknown; | ||
| resultUrl?: string; | ||
| }; | ||
|
|
||
| return { | ||
| components: { Node }, | ||
| setup() { | ||
| const app = useStoryApp(); | ||
| const pinia = createPinia(); | ||
| app.use(pinia); | ||
| setActivePinia(pinia); | ||
|
|
||
| const store = useStore(); | ||
| const baseNode = cloneNodeData(storyArgs.nodeData); | ||
| store.initData([baseNode], [], { loopType: "none" }); | ||
|
|
||
| const currentNode = computed(() => store.nodes[storyArgs.nodeIndex] ?? baseNode); | ||
|
|
||
| const handleUpdatePosition = (position: UpdateNodePositionData) => { | ||
| store.updateNodePosition(storyArgs.nodeIndex, position); | ||
| }; | ||
| const handleSavePosition = () => { | ||
| store.saveNodePositionData(); | ||
| }; | ||
| const handleStaticNodeValue = (payload: Record<string, unknown>) => { | ||
| store.updateStaticNodeValue(storyArgs.nodeIndex, payload as UpdateStaticValue, true); | ||
| }; | ||
| const handleNestedGraph = (payload: Record<string, unknown>) => { | ||
| store.updateNestedGraph(storyArgs.nodeIndex, payload as UpdateStaticValue); | ||
| }; | ||
|
|
||
| const onNewEdgeStart = action("newEdgeStart"); | ||
| const onNewEdge = action("newEdge"); | ||
| const onNewEdgeEnd = action("newEdgeEnd"); | ||
| const onOpenMenu = action("openNodeMenu"); | ||
| const onOpenEditMenu = action("openNodeEditMenu"); | ||
| const onDragStart = action("nodeDragStart"); | ||
| const onDragEnd = action("nodeDragEnd"); | ||
|
|
||
| if (Object.prototype.hasOwnProperty.call(storyArgs, "resultUrl")) { | ||
| const resultUrl = toRef(storyArgs, "resultUrl"); | ||
| watch( | ||
| resultUrl, | ||
| (url) => { | ||
| if (url) { | ||
| store.setResult(baseNode.nodeId, { url }); | ||
| return; | ||
| } | ||
| store.setResult(baseNode.nodeId, undefined); | ||
| }, | ||
| { immediate: true }, | ||
| ); | ||
| } | ||
|
|
||
| return { | ||
| args: storyArgs, | ||
| currentNode, | ||
| handleUpdatePosition, | ||
| handleSavePosition, | ||
| handleStaticNodeValue, | ||
| handleNestedGraph, | ||
| onNewEdgeStart, | ||
| onNewEdge, | ||
| onNewEdgeEnd, | ||
| onOpenMenu, | ||
| onOpenEditMenu, | ||
| onDragStart, | ||
| onDragEnd, | ||
| }; | ||
| }, | ||
| template: ` | ||
| <div class="relative h-[360px] w-[360px] bg-slate-900 p-6"> | ||
| <Node | ||
| :node-data="currentNode" | ||
| :node-index="args.nodeIndex" | ||
| :nearest-data="args.nearestData" | ||
| :is-connectable="args.isConnectable" | ||
| @update-position="handleUpdatePosition" | ||
| @save-position="handleSavePosition" | ||
| @update-static-node-value="handleStaticNodeValue" | ||
| @update-nested-graph="handleNestedGraph" | ||
| @new-edge-start="onNewEdgeStart" | ||
| @new-edge="onNewEdge" | ||
| @new-edge-end="onNewEdgeEnd" | ||
| @open-node-menu="onOpenMenu" | ||
| @open-node-edit-menu="onOpenEditMenu" | ||
| @node-drag-start="onDragStart" | ||
| @node-drag-end="onDragEnd" | ||
| /> | ||
| </div> | ||
| `, | ||
| }; | ||
| }; | ||
|
|
||
| const languageModelNode: GUINodeData = { | ||
| type: "computed", | ||
| nodeId: "assistant", | ||
| position: { x: 24, y: 24 }, | ||
| data: { | ||
| guiAgentId: "openAIAgent", | ||
| params: { | ||
| system: "You are a helpful assistant.", | ||
| stream: true, | ||
| temperature: 0.7, | ||
| }, | ||
| agentIndex: 0, | ||
| }, | ||
| }; | ||
|
|
||
| export const LanguageModelNode: Story = { | ||
| args: { | ||
| nodeData: languageModelNode, | ||
| nodeIndex: 0, | ||
| isConnectable: true, | ||
| }, | ||
| render: createRender(), | ||
| }; | ||
|
|
||
| const staticValueNode: GUINodeData = { | ||
| type: "static", | ||
| nodeId: "prompt", | ||
| position: { x: 24, y: 24 }, | ||
| data: { | ||
| staticNodeType: "text", | ||
| value: "Tell me a story about Storybook.", | ||
| }, | ||
| }; | ||
|
|
||
| export const StaticValueNode: Story = { | ||
| args: { | ||
| nodeData: staticValueNode, | ||
| nodeIndex: 0, | ||
| isConnectable: true, | ||
| }, | ||
| render: createRender(), | ||
| }; | ||
|
|
||
| const imageNode: GUINodeData = { | ||
| type: "computed", | ||
| nodeId: "imageGenerator", | ||
| position: { x: 24, y: 24 }, | ||
| data: { | ||
| guiAgentId: "openAIImageAgent", | ||
| params: { | ||
| prompt: "A neon llama visiting Storybook City", | ||
| }, | ||
| }, | ||
| }; | ||
|
|
||
| export const ImageResultNode: Story = { | ||
| args: { | ||
| nodeData: imageNode, | ||
| nodeIndex: 0, | ||
| isConnectable: true, | ||
| resultUrl: "https://placekitten.com/320/200", | ||
| } as any, | ||
| render: createRender(), | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Extract the duplicated render factory into a shared utility.
This file duplicates almost all logic from Node2.stories.ts (lines 29-130). The render factory pattern, cloneNodeData helper, and event handler setup are identical. This creates maintenance burden and violates the DRY principle.
Consider creating a shared utility in src/storybook/nodeStoryHelpers.ts:
// src/storybook/nodeStoryHelpers.ts
import type { StoryObj } from "@storybook/vue3";
import { action } from "@storybook/addon-actions";
import { computed, toRef, watch } from "vue";
import { createPinia, setActivePinia } from "pinia";
import { useStore } from "../store";
import type { GUINodeData, UpdateNodePositionData, UpdateStaticValue } from "../utils/gui/type";
import { useStoryApp } from "./useStoryApp";
export const cloneNodeData = (node: GUINodeData): GUINodeData =>
structuredClone(node);
export const createNodeRender = (Component: any): StoryObj["render"] =>
(args) => {
const storyArgs = args as {
nodeData: GUINodeData;
nodeIndex: number;
isConnectable: boolean;
nearestData?: unknown;
resultUrl?: string;
};
return {
components: { NodeComponent: Component },
setup() {
const app = useStoryApp();
const pinia = createPinia();
app.use(pinia);
setActivePinia(pinia);
const store = useStore();
const baseNode = cloneNodeData(storyArgs.nodeData);
store.initData([baseNode], [], { loopType: "none" });
const currentNode = computed(() => store.nodes[storyArgs.nodeIndex] ?? baseNode);
const handleUpdatePosition = (position: UpdateNodePositionData) => {
store.updateNodePosition(storyArgs.nodeIndex, position);
};
const handleSavePosition = () => {
store.saveNodePositionData();
};
const handleStaticNodeValue = (payload: Record<string, unknown>) => {
store.updateStaticNodeValue(storyArgs.nodeIndex, payload as UpdateStaticValue, true);
};
const handleNestedGraph = (payload: Record<string, unknown>) => {
store.updateNestedGraph(storyArgs.nodeIndex, payload as UpdateStaticValue);
};
const onNewEdgeStart = action("newEdgeStart");
const onNewEdge = action("newEdge");
const onNewEdgeEnd = action("newEdgeEnd");
const onOpenMenu = action("openNodeMenu");
const onOpenEditMenu = action("openNodeEditMenu");
const onDragStart = action("nodeDragStart");
const onDragEnd = action("nodeDragEnd");
if ("resultUrl" in storyArgs) {
const resultUrl = toRef(storyArgs, "resultUrl");
watch(
resultUrl,
(url) => {
if (url) {
store.setResult(baseNode.nodeId, { url });
return;
}
store.setResult(baseNode.nodeId, undefined);
},
{ immediate: true },
);
}
return {
args: storyArgs,
currentNode,
handleUpdatePosition,
handleSavePosition,
handleStaticNodeValue,
handleNestedGraph,
onNewEdgeStart,
onNewEdge,
onNewEdgeEnd,
onOpenMenu,
onOpenEditMenu,
onDragStart,
onDragEnd,
};
},
template: `
<div class="relative h-[360px] w-[360px] bg-slate-900 p-6">
<NodeComponent
:node-data="currentNode"
:node-index="args.nodeIndex"
:nearest-data="args.nearestData"
:is-connectable="args.isConnectable"
@update-position="handleUpdatePosition"
@save-position="handleSavePosition"
@update-static-node-value="handleStaticNodeValue"
@update-nested-graph="handleNestedGraph"
@new-edge-start="onNewEdgeStart"
@new-edge="onNewEdge"
@new-edge-end="onNewEdgeEnd"
@open-node-menu="onOpenMenu"
@open-node-edit-menu="onOpenEditMenu"
@node-drag-start="onDragStart"
@node-drag-end="onDragEnd"
/>
</div>
`,
};
};Then both story files can be simplified to:
import { createNodeRender } from "../storybook/nodeStoryHelpers";
import Node from "./Node";
// ... meta definition ...
const createRender = () => createNodeRender(Node);
// ... story definitions ...🤖 Prompt for AI Agents
In packages/grapys-vue/src/components/Node.stories.ts lines 1-195: the render
factory, cloneNodeData helper and event handler setup are duplicated with
Node2.stories.ts; extract them into src/storybook/nodeStoryHelpers.ts as two
exports (cloneNodeData using structuredClone and createNodeRender that accepts
the component and returns the render function), move imports used by the factory
(action, vue/composables, pinia, useStore, types, useStoryApp) into that file,
then replace the local helpers in both story files by importing createNodeRender
and cloneNodeData and wire createRender = () => createNodeRender(Node); ensure
the new helper uses a generic component name (e.g., NodeComponent) and preserves
the resultUrl watch logic and all event handlers.
| @@ -0,0 +1 @@ | |||
| game-dev.ts No newline at end of file | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Reconsider this symlink-based configuration pattern.
A file containing only "game-dev.ts" as a string is an unusual and fragile approach to configuration. According to the AI summary, this relies on CI creating symlinks.
This pattern has several issues:
- Not self-documenting (the purpose is unclear from the code)
- Fragile (breaks if symlinks aren't created)
- Doesn't work well across different environments/OSes
- Makes local development confusing
Consider more robust alternatives:
- Environment variables: Use Vite's
import.meta.envfor configuration - Build-time configuration: Use Vite's define or mode-specific config files
- Direct imports: Use Vite aliases to map imports directly without symlinks
Example using environment variables:
// src/config/index.ts
const projectId = import.meta.env.VITE_PROJECT_ID || 'game-dev';
export { projectId };Or using Vite aliases in vite.config.ts:
resolve: {
alias: {
'@/config/project': resolve(__dirname, `src/config/${process.env.PROJECT_NAME || 'game-dev'}.ts`)
}
}🤖 Prompt for AI Agents
In packages/grapys-vue/src/config/project.ts around line 1 there's a fragile
symlink-based config represented only by the string "game-dev"; replace this
with a self-contained, robust configuration: create a real config module (e.g.,
src/config/index.ts) that exports a project identifier read from
import.meta.env.VITE_PROJECT_ID with a default of "game-dev", update project
imports to reference that module, and remove the symlink-based file;
alternatively, if you prefer build-time mapping, add a Vite alias in
vite.config.ts that resolves '@/config/project' to the appropriate file based on
process.env.PROJECT_NAME, and update CI/local docs to explain using
VITE_PROJECT_ID or PROJECT_NAME for environment-specific configuration.
1 participant
目的
UI開発、テストのためにStorybookを導入
#54 #55 の後にマージ
Summary by CodeRabbit
New Features
Documentation
Build & Infrastructure