Fix promptfiddle build #2168
Merged
Fix promptfiddle build #2168
+105
−68
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed everything up to a15a336 in 1 minute and 11 seconds. Click for details.
- Reviewed
147lines of code in4files - Skipped
0files when reviewing. - Skipped posting
7draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. turbo.json:60
- Draft comment:
Reduced 'generate' dependencies to only '@boundaryml/baml#build'. Ensure no essential build dependency was inadvertently removed. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
2. typescript/apps/fiddle-web-app/app/[project_id]/_components/ProjectView.tsx:67
- Draft comment:
Changed main container layout from 'flex-row' to 'flex-col' with 'overflow-y-auto'. Verify this layout change is intended for all viewports. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
3. typescript/apps/fiddle-web-app/app/[project_id]/_components/ProjectView.tsx:149
- Draft comment:
Added 'overflow-y-auto' to the playground container. This helps manage vertical overflow in that section. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
4. typescript/apps/fiddle-web-app/vercel-build.sh:81
- Draft comment:
Added 'pnpm build' in the baml-schema-wasm directory to build the WASM component. Confirm this additional build step is needed. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
5. typescript/packages/ui/src/styles/globals.css:70
- Draft comment:
Adjusted dark theme '--border' opacity from 22% to 10% and '--input' from 20% to 15%. Ensure these new values match the design specifications. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
6. typescript/packages/ui/src/styles/globals.css:207
- Draft comment:
Reformatted '--vscode-button-hover-background' and related properties into a multi-line format. No functional changes expected. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
7. typescript/apps/fiddle-web-app/app/[project_id]/_components/ProjectView.tsx:66
- Draft comment:
Typographical note: In the comment, "firefox wont apply the background color for some reason..." should use an apostrophe for "won't". Consider changing it to "Firefox won't apply the background color..." for proper spelling. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative and focuses on a typographical error in a comment, which does not affect the functionality or logic of the code. It does not provide a suggestion for code improvement or address any potential issues with the code itself.
Workflow ID: wflow_gMllJp8UL68omUZX
You can customize 
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed a6d94c4 in 30 seconds. Click for details.
- Reviewed
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. typescript/apps/fiddle-web-app/vercel-build.sh:79
- Draft comment:
Changed directory to 'engine/baml-schema-wasm/web'. Please confirm that this relative path exists in all target environments. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_1OeoOG4GMfwFKxNf
You can customize
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed bd749c2 in 1 minute and 19 seconds. Click for details.
- Reviewed
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. typescript/apps/fiddle-web-app/vercel-build.sh:83
- Draft comment:
Changed 'cd ../../' to 'cd ../../../' to properly navigate back to the project root. Consider using pushd/popd or absolute paths for more robust directory navigation. - Reason this comment was not posted:
Comment looked like it was already resolved.
Workflow ID: wflow_ocZUUc3bMHcHxzO1
You can customize
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 135ee5b in 34 seconds. Click for details.
- Reviewed
12lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. typescript/apps/fiddle-web-app/package.json:19
- Draft comment:
New dependency '@gloo-ai/baml-schema-wasm-web' added. Ensure it’s required for the promptfiddle build and available in the workspace. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_54KUGl8mkHnpbVmt
You can customize
There was a problem hiding this comment.
Skipped PR review on fcd89ea because no changed files had a supported extension. If you think this was in error, please contact us and we'll fix it right away.
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 6bc7164 in 37 seconds. Click for details.
- Reviewed
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. typescript/apps/fiddle-web-app/vercel-build.sh:34
- Draft comment:
Cosmetic change: added a period to the comment for consistency. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_pWq9mmodNttDwSlt
You can customize
Codecov ReportAll modified and coverable lines are covered by tests ✅ 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Skipped PR review on 7f3964d because no changed files had a supported extension. If you think this was in error, please contact us and we'll fix it right away.
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed a556a6b in 35 seconds. Click for details.
- Reviewed
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. typescript/apps/fiddle-web-app/vercel-build.sh:31
- Draft comment:
Minor style update: removed period from the comment for consistency. Ensure all comments follow the same punctuation style across the script. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_MpgwboXojNFhFyxS
You can customize
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed 05054c3 in 1 minute and 36 seconds. Click for details.
- Reviewed
79lines of code in1files - Skipped
0files when reviewing. - Skipped posting
3draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. typescript/apps/fiddle-web-app/app/page.tsx:1
- Draft comment:
Remove unused import if BAMLProject is not used. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% The import is clearly added in the diff and does appear to be unused in the visible code. However, we need to consider that: 1) TypeScript would catch this automatically if it were truly unused 2) The type might be used implicitly through type inference 3) The loadProject or ProjectView components might require this type internally. I might be missing implicit type usage, and TypeScript's compiler would catch truly unused imports anyway. Since TypeScript already handles unused imports and this is a purely type-level import, this comment isn't providing valuable feedback that the build system wouldn't already catch. Delete the comment since it's pointing out something that would be caught by TypeScript's compiler anyway.
2. typescript/apps/fiddle-web-app/app/page.tsx:22
- Draft comment:
Avoid wrapping 'params' with Promise.resolve; it's already a Promise. - Reason this comment was not posted:
Comment was on unchanged code.
3. typescript/apps/fiddle-web-app/app/page.tsx:4
- Draft comment:
Ensure that ProjectView works with SSR; if it relies on client-only hooks, ssr: false might be needed. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =50%<= threshold50%The comment is asking the author to ensure that a component works with SSR, which is a form of asking for confirmation or verification. This violates the rule against asking the author to ensure behavior is intended or tested. However, it does provide a specific suggestion about using 'ssr: false', which could be useful.
Workflow ID: wflow_SCLvQRXuqDfbSrIG
You can customize
|
|
||
| // We don't need this since it's already part of layout.tsx | ||
| // export const metadata: Metadata = { | ||
| // title: 'Prompt Fiddle', | ||
| // description: '...', | ||
| // } | ||
|
|
||
| export default function Home({ | ||
| export default async function Home({ | ||
| searchParams, |
There was a problem hiding this comment.
The 'searchParams' parameter is not used; consider removing it if unnecessary.
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed ec1ed12 in 40 seconds. Click for details.
- Reviewed
48lines of code in1files - Skipped
0files when reviewing. - Skipped posting
2draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. typescript/packages/playground-common/src/baml_wasm_web/JotaiProvider.tsx:54
- Draft comment:
Extract the SSR fallback object into a utility to avoid duplication with sessionStore. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
2. typescript/packages/playground-common/src/baml_wasm_web/JotaiProvider.tsx:91
- Draft comment:
Consider refactoring the SSR fallback for sessionStore into a shared helper to reduce code duplication. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
Workflow ID: wflow_OlmJSC8TQxPJQxqx
You can customize
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 56806cb in 1 minute and 17 seconds. Click for details.
- Reviewed
147lines of code in10files - Skipped
0files when reviewing. - Skipped posting
10draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. typescript/apps/fiddle-web-app/app/PostHogPageView.tsx:14
- Draft comment:
Good: Added a 'typeof window !== "undefined"' check in the useEffect. This guards against SSR issues. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. typescript/apps/fiddle-web-app/app/[project_id]/_components/TopNavbar.tsx:47
- Draft comment:
Check: Early return on 'typeof window === "undefined"' ensures client-only code. Confirm that the finally block resets loading properly. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
3. typescript/apps/fiddle-web-app/app/_components/PosthogProvider.tsx:22
- Draft comment:
Good: The window check in the RB2BElement's effect prevents execution on the server side. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
4. typescript/apps/fiddle-web-app/app/embed/clientwrapper.tsx:51
- Draft comment:
Good: The onReset callback now safely checks if window exists before calling window.location.reload(). - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
5. typescript/apps/fiddle-web-app/hooks/command-s.tsx:7
- Draft comment:
Good: The useEffect now returns early if window is undefined, preventing keybinding code on the server. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
6. typescript/packages/playground-common/src/shared/baml-project-panel/Jotai.tsx:20
- Draft comment:
Good: The fallback storage object prevents errors when window is undefined. This safely simulates localStorage in SSR. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
7. typescript/packages/playground-common/src/shared/baml-project-panel/playground-panel/prompt-preview/test-panel/index.tsx:25
- Draft comment:
Suggestion: For consistency, consider also adding a window check in the second onReset callback (used for rendering view) similar to the first one. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%None
8. typescript/packages/ui/src/components/sidebar.tsx:97
- Draft comment:
Good: The keyboard shortcut listener in SidebarProvider is wrapped with a window check to ensure client-only execution. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
9. typescript/packages/ui/src/hooks/use-hide-on-scroll.tsx:15
- Draft comment:
Good: The event listener for scroll is safely registered only when window exists. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
10. typescript/packages/ui/src/hooks/use-mobile.tsx:11
- Draft comment:
Good: Properly guards against SSR by checking if window is defined before using matchMedia. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_TPgtotU9W5I9324P
You can customize
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed a118e3f in 39 seconds. Click for details.
- Reviewed
25lines of code in2files - Skipped
1files when reviewing. - Skipped posting
2draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. engine/baml-schema-wasm/web/package.json:10
- Draft comment:
Switching to '--release' is good, but update the inline comment if needed. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. typescript/apps/fiddle-web-app/package.json:19
- Draft comment:
Removed dependency '@gloo-ai/baml-schema-wasm-web'. Confirm this removal is intentional. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
Workflow ID: wflow_uPca7fwNhHRnhxri
You can customize
There was a problem hiding this comment.
Skipped PR review on 364c2f7 because no changed files had a supported extension. If you think this was in error, please contact us and we'll fix it right away.
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 71529b0 in 1 minute and 8 seconds. Click for details.
- Reviewed
12lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. typescript/apps/fiddle-web-app/app/page.tsx:11
- Draft comment:
The inserted empty comment (//) appears redundant. Remove if not needed. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% While the suggestion is technically correct - the empty comment line is redundant and serves no purpose - this is an extremely minor issue. The comment is about a changed line, but it's so trivial that it doesn't warrant a PR comment. This falls under the "Do NOT make comments that are obvious or unimportant" rule. The empty comment line could potentially be serving as visual spacing between code blocks, which might have some minor aesthetic value. Even if it's for spacing, using an empty comment instead of a blank line is poor practice and unnecessary. However, this is still too trivial to warrant a PR comment. Delete this comment. While technically correct, it's too trivial of an issue to warrant a PR comment.
Workflow ID: wflow_jArWU1nu1hAuvtbq
You can customize
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Important
Fixes build process and enhances SSR compatibility by updating dependencies and adding
windowchecks across multiple files.turbo.json: Simplifiedgeneratetask dependencies to only include@boundaryml/baml#build.vercel-build.sh: Updated build script to navigate toengine/baml-schema-wasm/webbefore building.typeof window !== 'undefined'checks inPostHogPageView.tsx,TopNavbar.tsx,PosthogProvider.tsx,clientwrapper.tsx,command-s.tsx,JotaiProvider.tsx,Jotai.tsx,test-panel/index.tsx,sidebar.tsx,use-hide-on-scroll.tsx, anduse-mobile.tsxto prevent execution of browser-specific code during SSR.ProjectView.tsx: Changed layout to useflex-coland addedoverflow-y-autofor better scrolling.globals.css: Adjusted--borderand--inputopacity values for better UI consistency.This description was created by
for 71529b0. You can customize this summary. It will automatically update as commits are pushed.