WIP: Acorn/253 submodule api fixed #297
Open
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
> Co-authored-by: Himanshu <[email protected]>
This reverts commit 60c667c.
![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 c11b127 in 2 minutes and 53 seconds
More details
- Looked at
2499lines of code in50files - Skipped
3files when reviewing. - Skipped posting
30drafted comments based on config settings.
1. extensions/vscode/src/util/messagingContext.tsx:85
- Draft comment:
Verify dependencies in useCallback. - 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. gui/vite.config.ts:19
- Draft comment:
Confirm alias configuration. - Reason this comment was not posted:
Confidence changes required:20%<= threshold50%
None
3. extensions/vscode/src/webviewProtocol.ts:116
- Draft comment:
Review error handling for missing message fields. - Reason this comment was not posted:
Comment was on unchanged code.
4. gui/src/index.css:5
- Draft comment:
Replace '::root' with ':root'. - 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. gui/src/pages/onboarding/Onboarding.tsx:25
- Draft comment:
Verify overlay unlock sequence. - Reason this comment was not posted:
Confidence changes required:30%<= threshold50%
None
6. extensions/vscode/src/util/messagingContext.tsx:226
- Draft comment:
Improve type specificity in messaging hook. - 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. extensions/vscode/src/stubs/remoteConfig.ts:107
- Draft comment:
Verify that multiplying remoteConfigSyncPeriod by 1000*60 produces the intended interval (i.e. minutes). Clarify units in variable naming or documentation. - 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. gui/src/inventory/pearSettings/PearSettings.tsx:55
- Draft comment:
Typo detected: 'border-solidd' should be 'border-solid'. - 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.
9. gui/src/context/IdeMessenger.ts:12
- Draft comment:
Consider using a more specific type for the global 'vscode' object instead of 'any' for improved type safety. - 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.
10. gui/src/integrations/creator/index.ts:246
- Draft comment:
Review usage of console.dir for logging; ensure that no sensitive data is leaked in production logs. - 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.
11. gui/src/util/messagingContext.tsx:136
- Draft comment:
Consider adding cancellation support or cleanup for pending responses in case a response never arrives. - 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 reasonable timeout mechanism that rejects promises and cleans up state after 10 seconds. While explicit cancellation could be an additional feature, the current implementation already handles the core concern of cleaning up pending responses. The comment seems to miss that this functionality largely exists.
Perhaps there could be value in explicit cancellation beyond timeouts, like if the component unmounts or the user cancels an operation manually.
While manual cancellation could be useful, the current timeout implementation handles the core cleanup needs, making this more of an optional feature request than a critical issue.
The comment should be removed since the code already implements timeout-based cleanup for pending responses, making the core suggestion redundant.
12. extensions/vscode/src/ContinueGUIWebviewViewProvider.ts:277
- Draft comment:
In the click event listener inside the overlay script (lines 275-281), 'vscode.postMessage' and 'vscode.commands.executeCommand' are used directly. Since 'window.vscode' is defined earlier, it's safer and more consistent to use 'window.vscode' here to avoid potential undefined errors. - 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.
13. extensions/vscode/src/PearAIApi.ts:19
- Draft comment:
Minor: Consider adding a newline at the end of the file to conform with common file formatting conventions. - 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 having a newline at the end of files is a common convention, this is exactly the kind of thing that should be enforced by linters and formatters, not code reviews. It's a trivial formatting issue that doesn't affect functionality. The rules specifically say not to comment on obvious things that would be caught by the build.
Maybe missing newlines could cause issues in some build systems or tools? Maybe this is more important than I think?
While newlines at end of file can be important, this is still exactly the kind of mechanical formatting issue that should be handled by automated tools, not human reviewers or review comments.
Delete this comment. File ending newlines should be handled by linters/formatters, not code review comments.
14. extensions/vscode/src/activation/activate.ts:141
- Draft comment:
There is a small inconsistency in naming: the comment on line 141 refers to 'PearAPP' while elsewhere the extension uses 'pearai'. Consider standardizing the naming to avoid confusion. - 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.
15. extensions/vscode/src/index.ts:1
- Draft comment:
There's a missing newline at the end of the file. Adding a newline at the end is a common best practice and can help avoid potential issues with some tools. - 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 having a newline at end of file is indeed a best practice, this kind of issue would typically be caught by linters, formatters, or the build process. It's a very minor issue that doesn't affect functionality. The comment doesn't suggest any critical code changes.
The comment is technically correct and follows good engineering practices. Missing newlines can cause issues in some scenarios.
While technically correct, this is exactly the kind of minor issue that should be handled by automated tools rather than manual review comments. It doesn't require human judgment.
Delete this comment as it's too minor and would typically be caught by automated tooling.
16. extensions/vscode/src/integrations/creator/index.ts:196
- Draft comment:
Typo alert: In the 'createTask' method, the comment says '// TODO: setup listeners in roo code'. It seems 'roo' should be 'our'. Please update this to avoid confusion. - 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 assumes "roo" is a typo, but given this is a new file in a codebase I don't have full context for, "roo" could very well be an intentional reference to a component name or internal system. There are other TODOs in the file that seem to reference real future work. The comment is making assumptions without full context.
I could be wrong - maybe it really is a typo. The word "roo" does look unusual.
Without clear evidence that this is actually a typo rather than an intentional reference, we should err on the side of assuming the author knows what they're referring to.
The comment should be deleted as it makes assumptions without sufficient context and could actually be incorrect if "roo" is an intentional reference.
17. gui/src/continue-styles.css:85
- Draft comment:
Typo found in the comment on line 85: 'default taildwind adapted to VSCode PearAI Theme'. It should be 'default tailwind adapted to VSCode PearAI Theme'. - 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 comment is technically correct about the typo, it's just a misspelling in a documentation comment. Comments are for developers to read, and the typo doesn't affect functionality. The meaning is still clear despite the typo. According to the rules, we should not make purely informative comments or comments that are unimportant.
The typo could potentially confuse future developers looking for Tailwind-related code. Documentation quality is important for maintainability.
While documentation quality matters, this typo is minor and obvious enough that it doesn't significantly impact code understanding. The rules explicitly state not to make unimportant comments.
Delete this comment as it's purely informative and doesn't affect code functionality. The typo is minor and doesn't warrant a PR comment.
18. gui/src/index.css:5
- Draft comment:
Typographical error: The ':root' selector is mistakenly written as '::root'. It should be corrected to ':root' to follow correct CSS syntax. - 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.
19. gui/src/integrations/perplexity/perplexitygui.tsx:124
- Draft comment:
The variable name 'OFFSET_HERUISTIC' seems to have a typo. It is likely intended to be 'OFFSET_HEURISTIC'. Consider updating the variable name for clarity. - 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.
20. gui/src/inventory/pearSettings/PearSettings.tsx:55
- Draft comment:
Typo found: 'border-solidd' should be 'border-solid'. Please fix the extra 'd'. - 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.
21. gui/src/inventory/pearSettings/PearSettings.tsx:61
- Draft comment:
Typo in class name: 'border-solidd' should be 'border-solid'. - 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.
22. gui/src/inventory/pearSettings/PearSettings.tsx:62
- Draft comment:
Typo: 'border-solidd' should be corrected to 'border-solid' on this line. - 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.
23. gui/src/inventory/pearSettings/PearSettings.tsx:115
- Draft comment:
Typographical error: 'border-solidd' in the SidebarSection should be 'border-solid'. - 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.
24. gui/src/inventory/pearSettings/PearSettings.tsx:145
- Draft comment:
Typo: 'border-solidd' should be 'border-solid' in the ContentArea component. - 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.
25. gui/src/pages/AddNewModel/ConfigureProvider.tsx:233
- Draft comment:
Typo: The comment reads "The WatsonX Authentication coukd be done by two different ways". It should be "could be done". - 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.
26. gui/src/pages/AddNewModel/ConfigureProvider.tsx:242
- Draft comment:
Typo: The toggle option two is labeled "Authenticate by crendentials". It should be "Authenticate by credentials". - 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.
27. gui/src/pages/gui.tsx:188
- Draft comment:
Typo: The constant OFFSET_HERUISTIC appears to be misspelled. It should likely be OFFSET_HEURISTIC. Please fix the spelling. - 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.
28. gui/src/pages/gui.tsx:338
- Draft comment:
Typo: The event name 'restFirstLaunchInGUI' might be a typo. If the intended behavior is to reset something, consider renaming it to 'resetFirstLaunchInGUI'. - 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.
29. gui/src/pages/help.tsx:128
- Draft comment:
It appears that the StyledLink styled component is missing its closing backtick (and possibly a semicolon) at the end of its template literal (after the @media rule). Please add the closing backtick (and semicolon if needed) to properly close the styled component definition. - 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.
30. gui/src/pages/onboarding/LocalOnboarding.tsx:170
- Draft comment:
There is an extra leading space in the template literal on line 170 for the chat model command ({ollama run ${DefaultLocalModels.Chat}}). Please remove the space so that the command displays consistently with the other commands. - 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_s3wBb1TmKgw7NHGl
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.
| try { | ||
| // Get workspace root | ||
| const workspaceRoot = vscode.workspace.workspaceFolders?.[0]?.uri.fsPath | ||
| console.log("Workspace root:", workspaceRoot) |
There was a problem hiding this comment.
Consider removing or minimizing debug console.log statements in production code.
Suggested change
| console.log("Workspace root:", workspaceRoot) |
gui/src/pages/creator/planEditor.tsx
Outdated
| <button | ||
| onClick={handleMakeIt} | ||
| disabled={!newProjectPlan.trim()} | ||
| className="flex cursor-pointer gap-2 rounded-md bg-indigo-600 px-6 py-2 text-sm font-medium text-white transition-colours duration-200 hover:bg-indigo-700 disabled:opacity-50 disabled:cursor-not-allowed" |
There was a problem hiding this comment.
Typo: The Tailwind CSS class on line 74 uses transition-colours. Tailwind typically expects transition-colors. Please correct if this was unintended.
Suggested change
| className="flex cursor-pointer gap-2 rounded-md bg-indigo-600 px-6 py-2 text-sm font-medium text-white transition-colours duration-200 hover:bg-indigo-700 disabled:opacity-50 disabled:cursor-not-allowed" | |
| className="flex cursor-pointer gap-2 rounded-md bg-indigo-600 px-6 py-2 text-sm font-medium text-white transition-colors duration-200 hover:bg-indigo-700 disabled:opacity-50 disabled:cursor-not-allowed" |
There was a problem hiding this comment.
❌ Changes requested. Incremental review on 03fe53e in 2 minutes and 6 seconds
More details
- Looked at
291lines of code in6files - Skipped
0files when reviewing. - Skipped posting
12drafted comments based on config settings.
1. core/core.ts:78
- Draft comment:
Changed messenger from private to readonly, exposing it publicly. Ensure this is intentional and documented to avoid unintended external modifications. - 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 messenger is a core communication channel used extensively within the class. However, making it readonly rather than private seems intentional - it allows read access while still preventing modification. The class implements IMessenger interface and needs to expose its messenger. The comment assumes this is unintentional but provides no evidence that public access is harmful.
I could be wrong about the intentionality - maybe this was an accidental change. The comment does point out a potential encapsulation issue.
The readonly modifier was likely chosen deliberately to expose the messenger while preventing modifications. The class needs to expose its messenger to implement the IMessenger interface. Without stronger evidence that this causes problems, we should trust the author's design choice.
Delete the comment. The change from private to readonly appears intentional for implementing the messenger interface, and there's no clear evidence that public read access is problematic.
2. core/core.ts:364
- Draft comment:
Removal of 'console.log(model)' improves cleanliness; consider using a logging abstraction if debugging is needed in production. - Reason this comment was not posted:
Comment looked like it was already resolved.
3. core/llm/llms/index.ts:193
- Draft comment:
Removal of the 'llmFromProviderAndOptions' function; verify that there are no external dependencies on this function. - 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 there are no external dependencies on a removed function. This falls under the rule of not asking the author to double-check things or ensure behavior is intended. The comment does not provide a specific suggestion or point out a specific issue with the code.
4. extensions/vscode/src/integrations/creator/index.ts:207
- Draft comment:
The assertion uses '||' to check messageId or messageType. If both are required, change to '&&'. - 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. core/core.ts:78
- Draft comment:
Changing messenger from private to readonly exposes it publicly. Confirm this is intended. - 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%
While the comment correctly identifies a visibility change, it falls into the category of asking for confirmation ("Confirm this is intended"). The PR author likely made this change intentionally. The comment doesn't identify any specific problems that would arise from this change. It's just asking the author to double-check their work.
The messenger property being public could potentially be a security or encapsulation concern if it wasn't intended. The comment is trying to prevent accidental exposure of internal state.
While encapsulation is important, the comment doesn't identify any specific issues - it just asks for confirmation. If there were actual security or architectural concerns, those should be stated directly. The PR author likely had a reason for making this change.
This comment should be deleted as it merely asks for confirmation without identifying any specific problems. It violates our rule against asking authors to confirm their intentions.
6. core/core.ts:364
- Draft comment:
Removal of debug console.log(model) is a positive cleanup. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is purely informative, as it simply states that the removal of a debug statement is a positive cleanup. It does not provide any actionable feedback or suggestions for improvement.
7. core/llm/llms/index.ts:193
- Draft comment:
Function 'llmFromProviderAndOptions' was removed. Ensure no external consumers rely on it. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
The comment is asking the author to ensure that no external consumers rely on a removed function. This is similar to asking the author to double-check or verify something, which is against the rules. The comment does not provide a specific suggestion or point out a specific issue with the code.
8. core/llm/llms/index.ts:107
- Draft comment:
File is missing a newline at the end; consider adding one for consistency. - 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.
9. extensions/vscode/src/activation/activate.ts:12
- Draft comment:
Exporting vscodeExtension as a module-level variable and simplifying the returned API object looks fine. Ensure no dependencies on the old NODE_ENV-based return. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
The comment is asking the author to ensure that there are no dependencies on the oldNODE_ENV-based return when making changes to thevscodeExtension. This is a request for confirmation or verification, which violates the rules. The comment does not provide a specific code suggestion or ask for a specific test to be written. Therefore, it should be removed.
10. extensions/vscode/src/extension.ts:41
- Draft comment:
Throwing an error if dynamicImportAndActivate returns undefined improves safety. Ensure extension.extension.core is always defined. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is asking the author to ensure that a certain variable is always defined. It doesn't provide a specific suggestion or point out a specific issue with the code. It falls under the category of asking the author to double-check something, which is against the rules.
11. core/llm/llms/index.ts:31
- Draft comment:
Typo alert: The import for 'Msty' on line 31 might be a typographical error. Please confirm whether 'Msty' is the intended module name or if it should be 'Misty' (or another correct spelling). - 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. extensions/vscode/src/PearAIApi.ts:24
- Draft comment:
The file is missing a newline at the end. Adding a newline at the end of the file is a common convention that helps avoid potential issues with some tools and improves diff readability. - 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 this is technically correct, the review rules state not to comment on anything that would be obviously caught by the build. Missing newlines at end of file are typically caught by linters and formatters which are part of the build process. This is a very minor issue that doesn't affect functionality.
Maybe some projects don't have linters configured to catch this? Maybe this is actually an important issue for some specific tools in this project?
Even if linters don't catch this, it's such a minor issue that it doesn't warrant a PR comment. The rules specifically say not to comment on obvious or unimportant issues.
This comment should be deleted as it points out a minor formatting issue that would typically be caught by build tools and doesn't affect functionality.
Workflow ID: wflow_JAbWX5qgitJRIBJC
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.
|
|
||
| constructor(core: Core) { | ||
| this.messenger = core.messenger; | ||
| // this.ideMessenger = new SubmoduleIdeMessenger(core); |
There was a problem hiding this comment.
Remove commented code if not needed.
Suggested change
| // this.ideMessenger = new SubmoduleIdeMessenger(core); |
|
|
||
| const gen = this._messenger.invoke( | ||
| "llm/streamChat", | ||
| { |
There was a problem hiding this comment.
Consider wiring up the cancelToken in llm/streamChat stream for improved responsiveness.
| const messages: ChatMessage[] = [ | ||
| // TODO: PROMPT INJECTION! | ||
| { | ||
| content: msg.payload.text, |
There was a problem hiding this comment.
Consider sanitizing msg.payload.text to mitigate prompt injection risks.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 27e1b38 in 1 minute and 56 seconds
More details
- Looked at
49lines of code in3files - Skipped
0files when reviewing. - Skipped posting
8drafted comments based on config settings.
1. core/llm/llms/PearAIServer.ts:165
- Draft comment:
Consider validating that 'prompt_key' isn't an empty string. If an empty value is passed, the header is omitted. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%
None
2. core/util/parameters.ts:38
- Draft comment:
Double-check that using localhost as SERVER_URL is intentional here (production URL is commented out). - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
The comment is asking the PR author to double-check their intention regarding the use of localhost as SERVER_URL. This violates the rule against asking the author to confirm their intention or to double-check things. Therefore, this comment should be removed.
3. extensions/vscode/src/integrations/creator/index.ts:127
- Draft comment:
Use of 'prompt_key' in completionOptions looks intentional; consider updating type definitions for 'CompletionOptions' for stronger typing. - Reason this comment was not posted:
Confidence changes required:40%<= threshold50%
None
4. core/llm/llms/PearAIServer.ts:162
- Draft comment:
Switched from 'creatorMode' boolean check to 'prompt_key'. Consider validating withtypeof options.prompt_key === 'string'to ensure correctness. - 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 current code uses 'in' operator and type assertion which is generally safe since TypeScript will catch type mismatches at compile time. The suggested change to typeof would add runtime type checking but may be redundant with TypeScript's static typing. Without seeing the type definitions for 'options', I can't be certain if this additional check is truly needed.
I might be overvaluing type safety - if the options object comes from TypeScript code, the additional runtime check may be unnecessary overhead. The current code may already be sufficiently safe.
While explicit type checking can be valuable, in a TypeScript codebase the static type system should already prevent invalid types. The current code is likely sufficient.
The comment suggests a change that would add redundant type checking in a TypeScript codebase. The current implementation is likely adequate.
5. core/llm/llms/PearAIServer.ts:176
- Draft comment:
Header now sends 'prompt_key' instead of 'creator-mode'; ensure backend support for this key and update any related docs. - Reason this comment was not posted:
Confidence changes required:40%<= threshold50%
None
6. core/util/parameters.ts:39
- Draft comment:
SERVER_URL is set to localhost now; confirm that this is intended for production or consider using an environment variable for flexibility. - 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 comment is asking for confirmation ("confirm that this is intended") which violates our rules. It's also suggesting an environment variable, but that's already acknowledged in the existing TODO comment. The change is likely intentional since they explicitly commented out the production URL.
The localhost change could be an accidental commit that would break production. Maybe we should keep the comment as a safeguard?
Per our rules, we should not ask for confirmations or make speculative comments. If this is a production issue, it would be caught during deployment or testing.
Delete the comment. It violates our rules by asking for confirmation and suggesting something that's already noted in the TODO.
7. extensions/vscode/src/integrations/creator/index.ts:128
- Draft comment:
Added prompt_key in completionOptions with 'creator_mode_plan'. Consider if this hard-coded value should be configurable and update the TODO if resolved. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%
None
8. extensions/vscode/src/integrations/creator/index.ts:159
- Draft comment:
Typographical error: In the comment on firing _onDidRequestExecutePlan (line 159), 'roo code extension' appears to be misspelled. It likely should be 'core code extension'. - 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_P0hBF4L3RWZthVBs
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 864de0e in 1 minute and 49 seconds
More details
- Looked at
359lines of code in5files - Skipped
1files when reviewing. - Skipped posting
10drafted comments based on config settings.
1. gui/package.json:47
- Draft comment:
Verify if the newly added 'motion' dependency is needed. 'framer-motion' is already used and may be sufficient. Consider removing potential duplicate dependencies. - 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 observation about two motion libraries is interesting, this comment violates several rules. It's asking to "verify" something, which is explicitly discouraged. It's also speculative ("may be sufficient") and about dependency management, which our rules say to ignore. We don't have enough context to know if both libraries serve different purposes.
The presence of two similar-sounding libraries could indicate a real issue with dependency bloat. Maybe this deserves investigation.
Per our rules, we should not comment on dependency changes or ask developers to verify things. We don't have strong evidence that this is actually problematic.
Delete the comment as it violates rules about dependency management and speculative verification requests.
2. gui/src/pages/creator/index.tsx:207
- Draft comment:
Add explicit typing for the 'msg' parameter in the handleAnimation callback for better type safety. - 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. gui/src/pages/creator/planEditor.tsx:9
- Draft comment:
Removal of the PlanningBar prop 'handleMakeIt' appears intentional. Confirm that upstream components calling PlanEditor no longer require this prop. - 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 that upstream components no longer require a removed prop. This is similar to asking for confirmation of intention, which is against the rules. The comment does not provide a specific suggestion or point out a specific issue with the code.
4. gui/src/pages/creator/ui/ideation.tsx:15
- Draft comment:
Good addition of optional 'className' prop. Ensure consistent usage of utility 'cn' across similar components. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%
None
5. gui/src/pages/creator/ui/planningBar.tsx:14
- Draft comment:
The PlanningBar component looks well-structured. No issues found. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%
None
6. gui/package.json:47
- Draft comment:
The added dependency 'motion' may be redundant since 'framer-motion' is already used. Please verify if it's required or remove it to avoid confusion. - 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%
While the comment points out a potential redundancy, I need to consider: 1) This is a dependency-related comment, which our rules explicitly say to ignore 2) We don't have context about whether these packages serve different purposes 3) The comment uses "Please verify" language which our rules say to avoid 4) The rules state not to comment on dependency changes.
The comment might be identifying a real issue with dependency bloat that could affect the project. Package redundancy can lead to maintenance problems.
While package redundancy is a valid concern, our rules explicitly state not to comment on dependency changes or library versions we don't recognize. We should trust the author's judgment on package choices.
The comment should be deleted as it violates our rules about not commenting on dependency changes and not asking authors to verify things.
7. gui/src/pages/creator/index.tsx:200
- Draft comment:
The dependency array for handleUserChangeMessage mistakenly uses setCurrentState instead of setMessages. It should depend on setMessages. - 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. gui/src/pages/creator/index.tsx:183
- Draft comment:
In handleLlmCall, the new assistant message is appended asynchronously, but sendMessage uses the stale 'messages' state from the closure. Consider refactoring to ensure the updated state is sent. - Reason this comment was not posted:
Comment was on unchanged code.
9. gui/src/pages/creator/planEditor.tsx:167
- Draft comment:
Using the array index as the key in the map() may lead to rendering issues if the messages order changes. Consider using a unique identifier for each message if available. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%
None
10. gui/src/pages/creator/index.tsx:261
- Draft comment:
Minor typo: The Tailwind CSS class for setting a font using an arbitrary value is written as 'font[var(--vscode-font-family)]'. It might be intended to be 'font-[var(--vscode-font-family)]' (with a hyphen after 'font'). Please check and update if necessary. - 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_9iOyGLzMypmhcryt
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
❌ Changes requested. Incremental review on 19df45b in 2 minutes and 12 seconds
More details
- Looked at
104lines of code in2files - Skipped
0files when reviewing. - Skipped posting
14drafted comments based on config settings.
1. gui/src/pages/creator/index.tsx:269
- Draft comment:
The restructuring of the IDEATION AnimatePresence container (moving the absolute div inside the AnimatePresence) may affect layout. Confirm this change is intentional. - Reason this comment was not posted:
Comment looked like it was already resolved.
2. gui/src/pages/creator/index.tsx:320
- Draft comment:
The container for the plan stage has been changed from an absolutely positioned div to a non-absolute one. Verify that removing the absolute position does not impact the intended overlay layout. - 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 impact of a change, which violates the rule against asking for confirmation or verification of intended behavior. It does not provide a specific suggestion or ask for a test to be written.
3. gui/src/pages/creator/index.tsx:331
- Draft comment:
PlanEditor container styling changed by replacing fixed height (h-[90vh]) with a bottom margin (mb-12). Confirm that removing the fixed height is intended for a more responsive design. - Reason this comment was not posted:
Comment looked like it was already resolved.
4. gui/src/pages/creator/inputBox/index.tsx:136
- Draft comment:
Added 'min-h-24' to the container div. Ensure that this Tailwind class exists and meets the design requirements. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%
None
5. gui/src/pages/creator/inputBox/index.tsx:149
- Draft comment:
The textarea now includes a 'min-h-5' class. Verify that this minimum height aligns with the design expectations and that the class is defined in your Tailwind configuration. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%
None
6. gui/src/pages/creator/index.tsx:214
- Draft comment:
Removed the unused handleUserChangeMessage callback. Confirm that the inline logic in PlanEditor now fully replaces its functionality. - Reason this comment was not posted:
Comment looked like it was already resolved.
7. gui/src/pages/creator/index.tsx:267
- Draft comment:
Added 'flex flex-col' to this container for vertical stacking. Please verify that the updated layout meets design expectations across screen sizes. - 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 layout across screen sizes, which is against the rules. It does not provide a specific code suggestion or highlight a potential issue with the code itself.
8. gui/src/pages/creator/index.tsx:270
- Draft comment:
The AnimatePresence wrapper has been restructured for the IDEATION state. Double-check that the inner 'absolute' container inside the conditional render maintains the intended layering and alignment. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is asking the PR author to double-check the layering and alignment after restructuring, which violates the rule against asking the author to confirm or double-check things. It does not provide a specific suggestion or point out a clear issue.
9. gui/src/pages/creator/index.tsx:320
- Draft comment:
The container for the plan editor has had its 'absolute' positioning removed. Ensure this change still meets overlay and alignment requirements. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
The comment is asking the PR author to ensure that the change meets certain requirements, which violates the rule against asking the author to ensure behavior is intended or tested. It does not provide a specific suggestion or ask for a specific test to be written.
10. gui/src/pages/creator/index.tsx:331
- Draft comment:
Replaced a fixed height ('h-[90vh]') with a bottom margin ('mb-12') for the PlanEditor. This likely improves responsiveness, but please verify that content does not unexpectedly overflow. - 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 the content does not overflow, which is against the rules. It does not provide a specific suggestion or point out a clear issue with the code change.
11. gui/src/pages/creator/index.tsx:347
- Draft comment:
The exit button container has been refactored to use a flex spacer and margin (mb-8) rather than absolute positioning. Confirm that the positioning works well on all target devices. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is asking the PR author to confirm the behavior of the UI change across different devices, which is a form of asking for confirmation or testing. This violates the rule against asking the author to confirm or ensure behavior is intended or tested.
12. gui/src/pages/creator/inputBox/index.tsx:136
- Draft comment:
Added the 'min-h-24' class to the input box container to enforce a minimum height. Ensure that this value is consistent with your design system (and that it's defined in Tailwind if customized). - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
The comment is asking the PR author to ensure consistency with the design system and Tailwind configuration. This falls under asking the author to double-check things, which is against the rules.
13. gui/src/pages/creator/index.tsx:232
- Draft comment:
Typo in comment: 'Register handlerx' should be corrected to 'Register handler'. - 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.
14. gui/src/pages/creator/inputBox/index.tsx:41
- Draft comment:
The comment on the default maxHeight property says "Default to 50vh" but the actual default value is '40vh'. Please update the comment to match the code to avoid confusion. - 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_gayNhU7uPuUd3Iy8
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.
| onChange={handleTextareaChange} | ||
| onKeyDown={handleTextareaKeyDown} | ||
| placeholder={placeholder} | ||
| className="w-full appearance-none bg-transparent outline-none focus:outline-none resize-none overflow-y-auto rounded-lg leading-normal py-2 px-2 flex items-center border-none p-2 pr-4 min-h-5" |
There was a problem hiding this comment.
Added 'min-h-5' to the textarea for a minimum height. Consider cleaning up duplicate padding classes (e.g. 'py-2 px-2' and 'p-2 pr-4') to avoid redundancy.
Suggested change
| className="w-full appearance-none bg-transparent outline-none focus:outline-none resize-none overflow-y-auto rounded-lg leading-normal py-2 px-2 flex items-center border-none p-2 pr-4 min-h-5" | |
| className="w-full appearance-none bg-transparent outline-none focus:outline-none resize-none overflow-y-auto rounded-lg leading-normal flex items-center border-none p-2 pr-4 min-h-5" |
…submodule (IDE) comms
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 14be7f4 in 1 minute and 56 seconds
More details
- Looked at
39lines of code in1files - Skipped
0files when reviewing. - Skipped posting
7drafted comments based on config settings.
1. gui/src/pages/creator/index.tsx:292
- Draft comment:
Removed extra AnimatePresence wrapper. Verify that the intended animation sequencing remains unaffected. - 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 the intended animation sequencing remains unaffected, which violates the rule against asking the author to confirm or verify intentions. It does not provide a specific suggestion or point out a specific issue.
2. gui/src/pages/creator/index.tsx:302
- Draft comment:
Reduced the duration for the planningBar animation from 0.4 to 0.3. Confirm this timing meets design intent. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%
None
3. gui/src/pages/creator/index.tsx:321
- Draft comment:
Removed scaleX transform from planEditor motion div. Ensure this omission is intentional and doesn't affect visual consistency. - Reason this comment was not posted:
Comment looked like it was already resolved.
4. gui/src/pages/creator/index.tsx:292
- Draft comment:
Duplicate AnimatePresence wrapper removed. Confirm that removing this extra wrapper doesn’t disrupt the intended exit animations for the IDEATION block. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%
None
5. gui/src/pages/creator/index.tsx:302
- Draft comment:
The transition duration for the planningBar has been updated from 0.4 to 0.3 seconds. Ensure this speed adjustment aligns with the design intent. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%
None
6. gui/src/pages/creator/index.tsx:321
- Draft comment:
In the PlanEditor motion.div, the scaleX and exit properties have been removed from the initial and animate states, yet the transition still contains a scaleX spring configuration. Remove the unused scaleX config or reinstate the scaleX animation if it’s intentional, and verify that omitting the exit animation is desired. - 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 comment is technically correct - there is an unused scaleX transition config. However, this is a UI animation detail that likely has no functional impact. The PR author deliberately removed the scaleX animation, suggesting it was an intentional change. The rules state we should not comment on pure frontend UI/styling changes.
The unused config could indicate a mistake that leads to wasted performance. Also, inconsistent animation code could make future maintenance more difficult.
While those are valid concerns, they are minor UI implementation details. The rules explicitly state not to comment on UI/styling changes and to assume the author made UI changes correctly.
Delete this comment as it relates purely to UI animation styling, which we should assume the author changed intentionally and correctly.
7. gui/src/pages/creator/index.tsx:233
- Draft comment:
Typographical error in comment: 'Register handlerx' should likely be 'Register handler' (or similar). - 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_8lGsNMNS7bWLGft5
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 66a739c in 1 minute and 7 seconds
More details
- Looked at
53lines of code in2files - Skipped
0files when reviewing. - Skipped posting
4drafted comments based on config settings.
1. gui/src/pages/creator/rgbBackground.tsx:14
- Draft comment:
Consider adding 'borderImageSlice: 1' to the inline style to ensure consistent border rendering across browsers. - 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 "1" at the end of the borderImage value is actually already specifying the slice value in shorthand. Adding borderImageSlice: 1 would be redundant since it's already specified in the borderImage property. The current implementation follows standard CSS practices for border-image.
Could there be browsers where the shorthand notation isn't well supported and an explicit borderImageSlice property would be more reliable?
The shorthand notation with the slice value at the end is well-established and widely supported. Modern browsers all handle this syntax correctly.
The comment should be deleted because it suggests adding something that's already effectively present in the code, just in a different (valid) format.
2. gui/src/pages/creator/ui/ideation.tsx:84
- Draft comment:
Removal of 'px-4' changes horizontal padding; verify if this altered layout is intended. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%
None
3. gui/src/pages/creator/rgbBackground.tsx:11
- Draft comment:
Consider extracting inline styles (like border & borderImage) into a CSS class or constant, and explicitly set borderImageSlice for clarity. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%
None
4. gui/src/pages/creator/ui/ideation.tsx:84
- Draft comment:
Verify that removing the 'px-4' class from RGBWrapper is an intentional design change, as it affects the horizontal padding. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%
None
Workflow ID: wflow_ddHyH690NmO3QDz6
You can teach Ellipsis what to look for with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
* Working progress * Working * Polishing * Added onboarding * Added cancel * WIP add to workspace * Progress * progress * Wording * Progress * Added ~ working * Added working * Added working * Added newProject flag * Polish * Added folder * fixes * new project ui * fixes * fixes * fix * Changed order * Progress * Added polishing --------- Co-authored-by: Himanshu <[email protected]>
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed 5869d41 in 1 minute and 49 seconds. Click for details.
- Reviewed
51lines of code in2files - Skipped
0files when reviewing. - Skipped posting
11draft 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. gui/src/pages/creator/inputBox/index.tsx:105
- Draft comment:
Adding a static className may override external styles. Confirm this is intentional. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%None
2. gui/src/pages/creator/ui/ideation.tsx:186
- Draft comment:
Ensure the conditionally applied className for FolderPlus matches design guidelines. - Reason this comment was not posted:
Confidence changes required:30%<= threshold50%None
3. gui/src/pages/creator/ui/ideation.tsx:196
- Draft comment:
Verify that the conditional className on FileText provides consistent visual feedback. - Reason this comment was not posted:
Confidence changes required:30%<= threshold50%None
4. gui/src/pages/creator/inputBox/index.tsx:184
- Draft comment:
Review the use of 'cursor-pointer' on both the Button and its inner div for consistency; consider consolidating if redundant. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
5. gui/src/pages/creator/ui/ideation.tsx:186
- Draft comment:
For clarity and consistency, consider using the imported 'cn' utility to conditionally set the icon's className instead of inline template literals. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
6. gui/src/pages/creator/ui/ideation.tsx:196
- Draft comment:
Similarly, use the 'cn' utility for the FileText icon to manage conditional styling, ensuring consistency with other components. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
7. gui/src/pages/creator/inputBox/index.tsx:143
- Draft comment:
Typographical error: The className has 'border-solidd'. Did you mean 'border-solid'? This should be fixed for consistency. - 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. gui/src/pages/creator/inputBox/index.tsx:156
- Draft comment:
Typographical error: The className includes 'border-solidd'. It should likely be 'border-solid'. - 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.
9. gui/src/pages/creator/inputBox/index.tsx:169
- Draft comment:
Typographical error: Found 'border-solidd' in the className. It should be corrected to 'border-solid'. - 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.
10. gui/src/pages/creator/ui/ideation.tsx:162
- Draft comment:
Typo found: 'border-solidd' should be corrected to 'border-solid'. - 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.
11. gui/src/pages/creator/ui/ideation.tsx:226
- Draft comment:
Typo found: 'border-solidd' should be corrected to 'border-solid'. - 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_Ss6f6bHSySDZlJG3
You can customize 
| onToggle?.(newToggled); | ||
| } : undefined} | ||
| {...rest} | ||
| className="rounded-lg p-1.5 cursor-pointer" |
There was a problem hiding this comment.
Consider merging the default className ('rounded-lg p-1.5 cursor-pointer') with any className passed via {...rest} rather than overriding it entirely.
Suggested change
| className="rounded-lg p-1.5 cursor-pointer" | |
| className={`rounded-lg p-1.5 cursor-pointer ${rest.className ?? ''}`} |
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 84a6ac1 in 2 minutes and 28 seconds. Click for details.
- Reviewed
113lines of code in2files - Skipped
0files when reviewing. - Skipped posting
12draft 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. extensions/vscode/src/integrations/creator/index.ts:158
- Draft comment:
Logging change: now outputs the whole payload rather than payload.text. Verify that this is intended and that the payload structure is consistent. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
2. extensions/vscode/src/integrations/creator/index.ts:160
- Draft comment:
The payload is constructed using msg.payload.request for both 'plan' and 'text', and also accesses msg.payload.request.newProject. Confirm if msg.payload.request is always an object; if it's a string in some cases, this may lead to unexpected undefined values. - 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 diff shows removal of string template literals around msg.payload.request, suggesting it was previously being coerced to string. The new code treats it both as a string and an object, which could be problematic. However, this isn't a new issue - it existed before. The comment is about the general structure, not about what changed. The comment identifies a real potential type safety issue, but it's not related to the actual changes in the diff which are just about string template literal removal. While the type safety concern is valid, we should only comment on issues introduced or modified by the changes in this PR. Delete the comment since it's discussing a pre-existing architectural issue rather than something changed in this PR.
3. gui/src/pages/creator/index.tsx:54
- Draft comment:
Default state for makeAPlan changed from true to false. Confirm that this is aligned with the desired UX for direct submission vs planning. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
4. gui/src/pages/creator/index.tsx:291
- Draft comment:
Replaced 'plan' field with 'request' in the SubmitPlan message payload. Ensure that both sender and receiver of the message are updated to use the 'request' field consistently. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
5. extensions/vscode/src/integrations/creator/index.ts:158
- Draft comment:
Avoid using console.dir for debug logging in production code. Consider using a structured logging mechanism or remove these logs. - Reason this comment was not posted:
Confidence changes required:30%<= threshold50%None
6. extensions/vscode/src/integrations/creator/index.ts:160
- Draft comment:
Potential bug: newProject is extracted as msg.payload.request.newProject. If request is a string, this will be undefined. Consider using msg.payload.newProject explicitly. - Reason this comment was not posted:
Comment was on unchanged code.
7. gui/src/pages/creator/index.tsx:54
- Draft comment:
Double-check the default value change for makeAPlan (from true to false) to ensure it aligns with intended UX behavior. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to double-check a change, which violates the rule against asking for confirmation or verification of intention. It does not provide a specific suggestion or point out a clear issue.
8. gui/src/pages/creator/index.tsx:105
- Draft comment:
The currentPlan useMemo now directly returns the last message's content, removing the previous plan marker parsing logic. Confirm that this simplification is intended. - Reason this comment was not posted:
Comment looked like it was already resolved.
9. gui/src/pages/creator/index.tsx:292
- Draft comment:
The payload key has changed from 'plan' to 'request' in the SubmitPlan message. Ensure the backend and extension API expect this change. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =40%<= threshold50%The comment is asking the author to ensure that the backend and extension API expect the change in the payload key. This is a request for confirmation of intention, which violates the rules. However, it does point out a specific change that could have significant implications, so it might be useful to highlight the change itself without asking for confirmation.
10. extensions/vscode/src/integrations/creator/index.ts:165
- Draft comment:
Typo detected in the comment: 'roo code extension' seems to be a mistake. Consider correcting it to 'root code extension' or 'core code extension' for clarity. - 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.
11. extensions/vscode/src/integrations/creator/index.ts:168
- Draft comment:
The TODO comment uses the informal term 'shizz'. It might be more clear to replace 'shizz' with a more conventional term such as 'stuff' or 'functionality'. - Reason this comment was not posted:
Comment was on unchanged code.
12. gui/src/pages/creator/index.tsx:322
- Draft comment:
Typo: In the comment on line 322, "Register handlerx" appears to include a typo. It might have been intended to say "Register handler". Please correct this for clarity. - 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_KlR0UEF6sCWcqzKV
You can customize
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed cff9138 in 1 minute and 38 seconds. Click for details.
- Reviewed
13lines 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. gui/src/pages/creator/index.tsx:105
- Draft comment:
Removed console.dir debug logs. Confirm that such logs are no longer needed for tracing debugging information. This is a good cleanup if the logs were only for temporary debugging. - Reason this comment was not posted:
Comment looked like it was already resolved.
2. gui/src/pages/creator/index.tsx:105
- Draft comment:
Removed unnecessary debug console.dir calls. If these logs might be useful, consider wrapping them with a debug flag or using a dedicated logging utility. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
3. gui/src/pages/creator/index.tsx:320
- Draft comment:
There's a typographical error in the comment: 'Register handlerx' should be corrected to 'Register handler'. This is a trivial/cosmetic error. - 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_U7XEssxzKsvePxWr
You can customize
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 53bdd58 in 1 minute and 42 seconds. Click for details.
- Reviewed
12lines 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. gui/src/pages/creator/index.tsx:347
- Draft comment:
Removed inline background style. Ensure this is intentional as it may affect the overlay's background appearance, especially for consistency with the VSCode sidebar theme. - Reason this comment was not posted:
Comment looked like it was already resolved.
2. gui/src/pages/creator/index.tsx:347
- Draft comment:
Ensure the removal of the inline 'background' style is intentional. Confirm that the background is now correctly applied via CSS classes or elsewhere to avoid unexpected UI issues. - 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 rules explicitly state "Do NOT comment on any pure frontend UI or styling changes, only logic. Assume that if the author changed the UI, they did it correctly." This is a pure UI styling change - removing a background color. Additionally, the comment starts with "Ensure..." which is another red flag per the rules. Maybe the background color is critical for functionality rather than just styling? Maybe removing it could cause text to be unreadable? Even if the background is important, the rules clearly state to trust the author on UI changes. They likely moved the background to CSS classes or verified it works without it. This comment should be deleted as it violates the rule about not commenting on UI styling changes and uses the "Ensure that..." pattern that the rules specifically call out as not useful.
3. gui/src/pages/creator/index.tsx:320
- Draft comment:
Typographical error: The comment says "Register handlerx". Please correct it to "Register handler" for clarity. - 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_dLKUho5jyXEpmmZj
You can customize
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 995b63f in 2 minutes and 8 seconds. Click for details.
- Reviewed
55lines of code in1files - Skipped
0files when reviewing. - Skipped posting
5draft 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. gui/src/pages/creator/index.tsx:105
- Draft comment:
Reintroducing the logic to search all messages for a plan is fine, but note that the previous fallback (using the last message’s content) has been removed. Ensure this change is intentional. - 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 is asking the author to "ensure this change is intentional" which violates our rules about not asking authors to verify their intentions. The change appears to be a deliberate refactoring from a simple approach to a more sophisticated one that specifically looks for plan markers. The old code was commented out, suggesting this was an intentional change. The comment does identify a real behavioral change in the code. Maybe this change could cause issues if plans were previously being passed without theplanmarkers? Even if there could be compatibility issues, asking the author to "ensure" something is not actionable. If there's a real issue, we should point it out directly with evidence. Delete this comment since it merely asks the author to verify their intention rather than identifying a specific problem that needs fixing.
2. gui/src/pages/creator/index.tsx:105
- Draft comment:
Good to see reverse iteration re-enabled for plan extraction. Ensure all messages follow the expected content structure (string or MessagePart array) to avoid silent failures. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
3. gui/src/pages/creator/index.tsx:121
- Draft comment:
The regex (/plan\s*([\s\S]*?)\s*/) is case-sensitive. Consider using the 'i' flag if plan markers might come in different cases. - 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 is looking for plan markers in a specific format. Since this is an internal system parsing its own output, the plan markers would be generated in a consistent format. There's no evidence that the markers might come in different cases. Making it case-insensitive could actually mask errors if something is generating malformed markers. The comment does identify a potential edge case. If different parts of the system generate plan markers inconsistently, this could cause problems. However, this is an internal parsing system where we control both the generation and parsing of markers. Case sensitivity helps enforce consistency and catch errors. The suggested change could hide bugs. Delete the comment. The case-sensitive regex is appropriate for parsing internal markers, and making it case-insensitive could mask errors.
4. gui/src/pages/creator/index.tsx:105
- Draft comment:
Consider extracting this plan extraction logic into a helper function if it will be reused elsewhere for improved maintainability. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
5. gui/src/pages/creator/index.tsx:319
- Draft comment:
The inline comment on line 319 contains a typographical error ('Register handlerx'). Consider correcting it to 'Register handler' to improve clarity. - 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_gtrjxa6ej41rMdup
You can customize
…nto acorn/253-submodule-api-fixed
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 9d0c93b in 1 minute and 7 seconds. Click for details.
- Reviewed
55lines 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. gui/src/pages/creator/index.tsx:105
- Draft comment:
Good refactoring: iterating messages in reverse to extract a plan from within ```plan markers is clear. Consider extracting the plan extraction logic into a helper function for improved readability and reusability. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
2. gui/src/pages/creator/index.tsx:105
- Draft comment:
The new reverse-loop logic cleanly extracts the plan from various content types. Consider extracting this extraction logic into a separate helper for better reusability and readability. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
3. gui/src/pages/creator/index.tsx:319
- Draft comment:
Typo: The comment 'Register handlerx' should probably be 'Register handler'. - 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_gjnd9Zu5PquWt1HQ
You can customize
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed 3d0a025 in 2 minutes and 3 seconds. Click for details.
- Reviewed
237lines of code in3files - Skipped
0files when reviewing. - Skipped posting
14draft 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. core/util/parameters.ts:38
- Draft comment:
SERVER_URL is now set to localhost for testing. In production, consider making it an env variable to avoid accidental misconfiguration. - 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 comment has two parts: noting the localhost change and suggesting env variables. The localhost observation is just stating what's obvious from the diff. The env variable suggestion is already present in the existing TODO comment. This makes the automated comment redundant and not adding new value. The comment could be helpful as a stronger reminder that using localhost in production could be dangerous. Maybe the TODO has been ignored for a while? While the security concern is valid, we should trust that developers understand the implications of localhost vs production URLs. The existing TODO already captures the need for improvement. The comment should be deleted as it's redundant with the existing TODO comment and partly just states the obvious change.
2. gui/src/pages/creator/index.tsx:268
- Draft comment:
safePath is correctly memoized and sanitizes the project name. Consider handling empty paths or missing values to avoid concatenating with an empty string. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
3. gui/src/pages/creator/index.tsx:337
- Draft comment:
The debugging console.dir call inside handleIdeationRequest could be removed or replaced with proper logging before production. - 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% While console.dir calls should generally be removed before production, this seems like a minor code quality issue. The code is actively being developed and debugging statements may still be useful. The comment doesn't highlight a bug or serious issue. There are also other console.dir calls throughout the file that weren't commented on. The comment is technically correct - debug logging should be cleaned up. But is this important enough to flag in a PR review? Should we wait until a cleanup pass? While valid, this type of minor cleanup comment creates noise in PR reviews. These kinds of issues are better handled in a dedicated cleanup pass or with automated tools/linting. Delete this comment. While technically correct, it's too minor of an issue to flag in PR review and creates unnecessary noise.
4. gui/src/pages/creator/index.tsx:299
- Draft comment:
Dependency array for handleMakeIt was updated to use safePath. Ensure safePath always reflects the expected folder path, especially when projectConfig fields are empty. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
5. gui/src/pages/creator/ui/ideation.tsx:70
- Draft comment:
The useEffect sets a default project config when isCreatingProject or missing workspace folders. Verify that the hardcoded defaults (e.g., '~/pearai-projects' and 'default') are acceptable for all cases. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the author to verify if the hardcoded defaults are acceptable for all cases. This falls under asking the author to ensure the behavior is intended, which is against the rules. Therefore, this comment should be removed.
6. gui/src/pages/creator/ui/ideation.tsx:181
- Draft comment:
The input placeholder and button toggling now depend on isCreatingProject instead of the removed isPopoverOpen, which is good. Double-check that the logic meets UX expectations in all states. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
7. gui/src/pages/creator/index.tsx:268
- Draft comment:
Ensure that safePath handles cases where projectConfig.path might be empty. Consider adding a validation to avoid constructing invalid paths. - 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 validation in place - both usage sites check for projectConfig.path being truthy before using safePath. Adding validation inside safePath would be redundant since the callers are already handling this case. The current implementation actually handles empty paths fine - it would just join with the safeName which is acceptable behavior. I could be missing other usage sites of safePath that don't have validation. The current behavior of joining empty path with safeName might not be the intended behavior. Looking at the full file, safePath is only used in handleMakeIt and handleIdeationRequest, both of which have proper validation. The joining behavior is consistent with path joining conventions. Delete the comment. The suggested validation is redundant since the calling code already handles empty paths appropriately.
8. gui/src/pages/creator/index.tsx:337
- Draft comment:
The asynchronous folder creation in handleIdeationRequest is dispatched without awaiting. Verify if subsequent operations depend on folder creation and consider awaiting if 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% Looking at the code flow: 1. handleIdeationRequest first calls handleLlmCall which starts the LLM processing 2. Then optionally creates a folder as a side effect 3. The comment in the code says "Create the project folder now so roo code doesn't refresh" 4. Nothing appears to depend on the folder creation completing 5. The actual project creation happens later in handleMakeIt I could be missing some race condition between folder creation and subsequent operations. The error handling with .catch() suggests the developers considered failure cases. The code comment and implementation suggest this is intentionally not awaited - it's a side effect that doesn't need to block the main flow. Error handling is in place. This comment should be deleted. The non-awaited folder creation appears intentional and properly error-handled, with no evidence of dependent operations requiring it to complete first.
9. gui/src/pages/creator/ui/ideation.tsx:165
- Draft comment:
Typo in class names: 'border-solidd' should be 'border-solid'. - 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.
10. gui/src/pages/creator/ui/ideation.tsx:69
- Draft comment:
The effect resetting projectConfig when isCreatingProject is true (or no workspace folders) might override user input. Confirm that this behavior matches the intended UX. - Reason this comment was not posted:
Comment was on unchanged code.
11. core/util/parameters.ts:37
- Draft comment:
Typographical note: Consider updating 'Todo:' to 'TODO:' for consistency with common conventions. Also, if appropriate, consider capitalizing 'digitalocean' to 'DigitalOcean' to reflect proper branding. - Reason this comment was not posted:
Comment was on unchanged code.
12. gui/src/pages/creator/index.tsx:342
- Draft comment:
Typo found: the comment says "so roo code doesn't refresh". Please change "roo" to "our" to improve clarity. - 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% While the comment is technically correct - there is a typo in the comment - fixing typos in comments is extremely low value. Comments are for human readability and this typo doesn't significantly impact understanding. The meaning is still clear from context. This feels like unnecessary nitpicking. The typo could be seen as unprofessional and it's a very simple fix that would improve code quality slightly. The benefit is extremely minimal and not worth the overhead of a code review comment. We should focus review comments on more substantive issues. Delete this comment as it's too minor of an issue to be worth raising in code review.
13. gui/src/pages/creator/index.tsx:317
- Draft comment:
Typo in comment: "Register handlerx" should likely be "Register handler". - 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.
14. gui/src/pages/creator/ui/ideation.tsx:165
- Draft comment:
Typographical error: 'border-solidd' should be corrected to 'border-solid' at line 165. Also, similar correction is needed at line 229 where 'border-solidd' appears. - 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_wfTnI9BU1WIFvXgG
You can customize
| export const SERVER_URL = "https://server.trypear.ai/pearai-server-api2"; | ||
| // export const SERVER_URL = "http://localhost:8000"; | ||
| // export const SERVER_URL = "https://server.trypear.ai/pearai-server-api2"; | ||
| export const SERVER_URL = "http://localhost:8000"; |
There was a problem hiding this comment.
Avoid hardcoding the localhost URL in production. Use an environment variable instead.
Suggested change
| export const SERVER_URL = "http://localhost:8000"; | |
| export const SERVER_URL = process.env.SERVER_URL || "http://localhost:8000"; |
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 ✏️
Closes #xxx
What changed? Feel free to be brief.
Checklist ✅
Important
Introduces PearAI Creator with new UI components, messaging context, and styling, enhancing the user interface and interaction for the feature.
PearAI Creatorwith components likePlanEditor,Ideation,PlanningBar, andRGBWrapper.ColorManagerfor theme color updates.messagingContext.tsxfor webview communication.InputBoxfor user input with customizable buttons.buttonVariantsinbutton/index.tsxfor button styling.pearIcon.tsxfor PearAI branding.index.cssfor animations and styling specific to PearAI Creator.continue-styles.cssfor global styling.vite.config.tsfor build output and path aliasing.vite-env.d.tsfor global window properties.utils/index.tsfor animation utilities.This description was created by
for 3d0a025. You can customize this summary. It will automatically update as commits are pushed.