Web version pt.8 #1782
Merged
Web version pt.8 #1782
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
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
❌ Changes requested. Reviewed everything up to 59f96fa in 3 minutes and 46 seconds
More details
- Looked at
4669lines of code in25files - Skipped
1files when reviewing. - Skipped posting
15drafted comments based on config settings.
1. apps/web/client/.env.example:1
- Draft comment:
New env variable NEXT_CSB_API_KEY added. Ensure docs update about secure key management & that this key is not committed in production. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
The comment is asking the author to ensure documentation updates and secure key management, which falls under the category of asking the author to ensure something is done. This violates the rule against asking the author to ensure things are done.
2. apps/web/client/src/app/_components/csb.tsx:1
- Draft comment:
New CSB component uses TRPC csb procedures. Verify API endpoints are secure and error-handled. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
The comment is asking the PR author to verify that API endpoints are secure and error-handled, which falls under asking the author to ensure behavior is intended or tested. This violates the rules.
3. apps/web/client/src/app/_components/hello.tsx:1
- Draft comment:
File 'hello.tsx' was removed. Confirm that its removal is intentional and no clients depend on it. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
4. apps/web/client/src/app/_components/editor-bar/index.tsx:1
- Draft comment:
EditorBar component parameter 'selectedElement' removed; now hard-coded as 'div'. Confirm this change is intended. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
5. apps/web/client/src/app/project/[id]/page.tsx:1
- Draft comment:
Page now is an async server component that creates a dummy project. Ensure that the intended integration with a real Supabase fetch is implemented later. - 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 code already has a TODO comment on line 9 that clearly indicates the need to implement Supabase integration. The automated comment is just restating what's already explicitly marked in the code. Additionally, it's asking to "ensure" something happens later, which violates our rule about verification requests.
Maybe this comment provides valuable context about the architectural direction of using Supabase that isn't obvious from just the TODO?
No, the code already imports Supabase client and has a commented example query - the intention to use Supabase is clear from the code itself.
Delete the comment as it's redundant with the existing TODO comment and asks for verification of future work.
6. apps/web/client/src/components/store/index.ts:1
- Draft comment:
ProjectsManager replaced by ProjectManager. Confirm that UI usages (via useProjectsManager) are consistent. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
The comment is asking the author to confirm consistency in UI usages after a name change. This is a request for confirmation, which violates the rule against asking the author to confirm their intention or double-check things.
7. apps/web/client/src/server/api/routers/csb.ts:1
- Draft comment:
csbRouter is defined with start/stop/list. Verify that the input validation (z.string()) suffices. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
The comment is asking the PR author to verify the sufficiency of input validation, which violates the rule against asking the author to confirm or verify things. It does not provide a specific suggestion or point out a clear issue.
8. packages/vfs/package.json:1
- Draft comment:
New virtual FS package added. Validate that its dependencies (globby, isomorphic-git) meet requirements for your VFS use-case. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
The comment is asking the PR author to validate dependencies, which is against the rules. It doesn't provide a specific suggestion or point out a specific issue with the code.
9. packages/vfs/src/vfs.ts:1
- Draft comment:
VFS class provides stubs for sandbox, shell, and ports; ensure that implementation details are later filled in. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is purely informative and does not provide a specific suggestion or question related to the code changes. It violates the rule against making purely informative comments.
10. packages/vfs/test/git.test.ts:1
- Draft comment:
Extensive git tests are added for the VFS functionality via isomorphic-git. Verify the tests run reliably in the testing environment. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is asking the PR author to verify that tests run reliably, which is against the rules. It does not provide a specific code suggestion or point out a specific issue with the code.
11. apps/web/preload/package.json:1
- Draft comment:
Updated nanoid version order: check if version bump to nanoid ^5.1.5 is compatible with other packages. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
12. bun.lock:8
- Draft comment:
Large changes in bun.lock are present. Confirm that lock file regeneration and upgrades are intentional and tested for compatibility. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
The comment is asking the PR author to confirm their intention and ensure testing, which violates the rules. It also relates to dependency changes, which should not be commented on.
13. apps/web/client/src/components/store/projects/vps.ts:1
- Draft comment:
Typographical Error: The filename 'vps.ts' suggests that the class should perhaps be named 'VPS' rather than 'VFS'. Please ensure the class name matches the intended naming convention or update the filename accordingly. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50%
The naming inconsistency could indicate a typo, but without more context about what VFS or VPS stands for, I can't be certain this is actually a mistake. VFS could stand for "Virtual File System" while VPS could be "Virtual Private Server" - both would be reasonable in a project context. We shouldn't assume the author made a mistake without stronger evidence.
I might be too hesitant - inconsistent naming can lead to confusion and maintenance issues, so maybe this is worth flagging regardless of what the acronyms stand for.
However, without knowing the intended meaning of these acronyms, this comment could be wrong and waste the author's time. The author likely chose these names deliberately.
The comment should be deleted as it makes assumptions about naming without sufficient context to know if it's actually an issue.
14. apps/web/client/src/utils/codesandbox/server.ts:16
- Draft comment:
Minor typographical note: It looks like there is a missing semicolon at the end of the line 'const listResponse = await sdk.sandbox.list()'. For consistency and clarity, consider adding a semicolon at the end of the statement. - 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%
This is a style preference issue. The code will work perfectly fine without the semicolon. The file seems to use semicolons in some places but not others, suggesting there's no strict style enforcement. This kind of minor stylistic issue should be handled by a linter or code formatter, not manual code review comments.
Maybe there's a team-wide style guide that strictly requires semicolons that I'm not aware of? The inconsistency in semicolon usage within the file could be worth addressing.
Even if there is a style guide, this kind of formatting issue should be caught and fixed automatically by tools like ESLint or Prettier, not through manual code review comments.
Delete this comment. Semicolon usage is a minor style issue that should be handled by automated tools, not manual code review.
15. packages/vfs/src/vfs.ts:20
- Draft comment:
Typo: The doc comment on line 20 uses curly quotes (‘bash’) instead of standard straight quotes ('bash'). Consider replacing with standard quotes for consistency. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50%
The comment appears to be factually incorrect - the quotes in the doc comment are actually straight quotes, not curly quotes. Even if they were curly quotes, this would be a very minor stylistic issue that doesn't affect functionality. This is a new file, but the comment is about something that doesn't need to be changed.
Could I be missing some subtle rendering issue where the quotes appear different in certain contexts? Could quote style consistency be more important than I think?
No - the quotes are clearly straight quotes in the source, and even if they weren't, this would be too minor an issue to warrant a PR comment.
The comment should be deleted because it is factually incorrect and even if it weren't, it would be too minor an issue to raise.
Workflow ID: wflow_jtqaETLjPBRi8ZiE
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
| @@ -0,0 +1,19 @@ | |||
| import { CodeSandbox } from "@codesandbox/sdk"; | |||
| const sdk = new CodeSandbox(process.env.CSB_API_KEY!); | |||
There was a problem hiding this comment.
The CodeSandbox constructor uses process.env.CSB_API_KEY! without runtime check; consider a validation or fallback error to avoid runtime errors if missing.
| <div className="h-screen w-screen flex flex-row select-none relative" > | ||
| <Canvas /> | ||
|
|
||
| < div className="absolute top-0 w-full" > |
There was a problem hiding this comment.
There are extra spaces before the <div> tag in several places (e.g., < div ...> on lines 29, 33, 46, and 50). Please remove the space to make it <div ...>.
Suggested change
| < div className="absolute top-0 w-full" > | |
| <div className="absolute top-0 w-full" > |
| // export const useUpdateManager = () => useContext(UpdateContext); | ||
| export const useProjectsManager = () => useContext(ProjectContext); |
There was a problem hiding this comment.
Naming inconsistency: The hook is named useProjectsManager (plural) while the underlying instance and context are named using the singular form (ProjectManager and ProjectContext). Consider renaming the hook to useProjectManager or updating the instance/context names to maintain consistency.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on a1a865b in 1 minute and 0 seconds
More details
- Looked at
354lines of code in4files - Skipped
1files when reviewing. - Skipped posting
8drafted comments based on config settings.
1. packages/vfs/package.json:1
- Draft comment:
This deletion removes the entire vfs package. Is this intentional, given the PR is expected to add onboarding docs? - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
2. packages/vfs/src/index.ts:1
- Draft comment:
Removal of index.ts export may break module imports. Confirm if this removal is intended. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
3. packages/vfs/src/vfs.ts:1
- Draft comment:
vfs.ts is completely removed. Check if removal of VFS functionality is desired or an oversight. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
4. packages/vfs/test/git.test.ts:1
- Draft comment:
All tests for git functionality in vfs are deleted. This might impact test coverage. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
5. packages/vfs/package.json:1
- Draft comment:
Removed vfs package file. This removal does not align with the issue description (onboarding docs for Next.js, Remix, Webpack, Babel). Confirm if removing the package is intentional. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
6. packages/vfs/src/index.ts:1
- Draft comment:
The index export file is completely removed. Ensure this removal is deliberate, as no new onboarding documentation is added. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
7. packages/vfs/src/vfs.ts:1
- Draft comment:
The entire VFS implementation has been removed. Check if deprecating this module is intended, as it doesn't contribute to the new onboarding docs. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
8. packages/vfs/test/git.test.ts:1
- Draft comment:
All Git integration tests have been removed. Verify that this change is expected and that removal of these tests doesn't inadvertently reduce test coverage when adding new features. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_IvB9jI8mxILgGbzF
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
ml-delaurier
pushed a commit
to ml-delaurier/nolook
that referenced
this pull request
Apr 23, 2025
* Update nanoid * test with codesandbox * Working trpc test * Add project management and vps * Clear vfs package
t1c1
pushed a commit
to t1c1/onlookbotcodes
that referenced
this pull request
Jun 5, 2025
* Update nanoid * test with codesandbox * Working trpc test * Add project management and vps * Clear vfs package
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.
Description
Related Issues
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Important
Add CodeSandbox integration (start/stop/list sandboxes), project state management, and related UI/API/store refactors to the web client.
@codesandbox/sdk) topackage.jsonandbun.lock.NEXT_CSB_API_KEYto.env.examplefor CSB API authentication.CsbReact component insrc/app/_components/csb.tsxfor starting, stopping, and listing CSB sandboxes via tRPC mutations.csbRoutertosrc/server/api/routers/csb.tsand exposes it inappRouterfor CSB operations (start,stop,list).src/utils/codesandbox/server.tsfor sandbox management.ProjectManagerclass insrc/components/store/projects/index.tsfor managing project state and creation.VFSstub insrc/components/store/projects/vps.tsfor future virtual file system operations.useProjectsManagerReact context hook insrc/components/store/index.ts.src/app/project/[id]/page.tsxto fetch and pass aProjectobject to a newMaincomponent.src/app/project/[id]/_components/main.tsxto initialize and provide project state viauseProjectsManager.EditorBarinsrc/app/project/[id]/_components/editor-bar/index.tsxto remove props and use a hardcoded selection.src/app/_components/main.tsx) to include the newCsbcomponent.bun.lockfiles from multiple locations.nanoidversion inapps/web/preload/package.json.bun.lockfor consistency.hello.tsxcomponent.This description was created by
for a1a865b. It will automatically update as commits are pushed.