-
Notifications
You must be signed in to change notification settings - Fork 2.3k
enhancement: migrate handbook and janv2 #6552
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Caution
Changes requested ❌
Reviewed everything up to d2c8680 in 1 minute and 59 seconds. Click for details.
- Reviewed
3536lines of code in36files - Skipped
29files when reviewing. - Skipped posting
9draft 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. docs/src/components/Blog/index.tsx:27
- Draft comment:
Typo in the class name: '!fqont-normal' appears to be a typo and likely should be '!font-normal'. - Reason this comment was not posted:
Comment was on unchanged code.
2. docs/src/components/Navbar.tsx:3
- Draft comment:
Consider using 'next/navigation' instead of 'next/router' for consistency with other components if you are using Next 13’s new app directory. - 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% 1. We don't know if this project uses or plans to use Next.js 13 app directory. 2. Both next/router and next/navigation are valid approaches. 3. This is more of a preference/consistency suggestion without clear evidence that there's an actual problem. 4. The comment uses speculative language ("if you are using...") which violates our rules. The comment could be valid if we had evidence that other components use next/navigation or if project standards require it. Without seeing other files or knowing project standards, this is a speculative suggestion rather than pointing out a clear issue. Delete the comment as it's speculative and requires more context about project standards that we don't have access to.
3. docs/src/pages/index.mdx:6
- Draft comment:
Consider quoting each keyword in the YAML keywords array for consistency and to avoid potential parsing issues. - Reason this comment was not posted:
Comment was on unchanged code.
4. docs/src/pages/index.mdx:21
- Draft comment:
Static props fetching from GitHub API endpoints lacks error handling. Consider adding error handling or fallbacks to prevent build failures. - 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. docs/src/components/Blog/index.tsx:27
- Draft comment:
Typographical error: It appears!fqont-normalshould be replaced with!font-normal. - Reason this comment was not posted:
Comment was on unchanged code.
6. docs/src/components/FavoriteModels.tsx:14
- Draft comment:
Typo: The imported file name 'Amazone.svg' seems inconsistent with the app name 'Amazon'. Consider renaming the file or correcting the import. - 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 this is technically a typo, it's not a functional issue - the code will still work as long as the file exists with that name. The rules say not to comment on file renames or anything that would be caught by the build. If the file was actually named incorrectly, the build would fail. If the file exists with the 'Amazone.svg' name, this is just a style preference. The file might actually be named 'Amazone.svg' intentionally, perhaps due to some naming convention or to avoid conflicts. Without seeing the actual files in the assets directory, we can't be certain this is a mistake. Even if intentional, the inconsistency between the import name and usage is confusing and could cause maintenance issues later. However, this falls into the category of things that would be caught by the build if it was actually wrong. Delete the comment. While it points out a potential inconsistency, it falls under the rule about not commenting on file renames or things that would be caught by the build system.
7. docs/src/components/FavoriteModels.tsx:53
- Draft comment:
Typo: In the 'thingsToRemember' array, the item 'Minimalist UI tasted' appears to be a typo. Please verify the intended phrasing. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 30% vs. threshold = 50% While "tasted" seems unusual, it could be an intentional play on words or reference to having experienced/tried minimalist UI. Without more context about the user "Joe" and their preferences, I can't be 100% certain this is a typo. The comment is suggesting a change but isn't completely confident either. I might be overthinking this - "tasted" in this context does read awkwardly and "taste" would be the more natural word choice. But I should be careful about assuming what's "natural" in UI copy. Even if this is likely a typo, the rules state we should only keep comments when we have strong evidence of an issue. Without being able to confirm the intended meaning, we should err on the side of not commenting. Delete the comment since we don't have strong evidence that "tasted" is definitely wrong, even though it seems unusual.
8. docs/src/components/FooterMenu/index.tsx:121
- Draft comment:
Typographical suggestion: In the Tailwind CSS class for the button, consider adding spaces in the calc() function. For readability and to follow CSS conventions, replaceh-[calc(100%-8px)]withh-[calc(100% - 8px)]. - 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% This is a purely stylistic change about whitespace in a CSS calc function. While technically correct CSS convention, it's an extremely minor detail that doesn't affect functionality. The current code works fine either way. This falls under being too obvious/unimportant to warrant a PR comment. The spacing convention in calc() functions is a real CSS best practice that can improve readability. Maybe this could help prevent future inconsistencies in the codebase? While it's a valid CSS convention, this level of nitpicking about spaces in a single CSS calc() instance isn't worth the cognitive overhead of a PR comment. Style consistency at this micro level can be better handled by linters or style guides. Delete this comment. It's too minor of a stylistic suggestion to warrant a PR comment and doesn't meaningfully impact code quality or functionality.
9. docs/src/pages/handbook/why.mdx:56
- Draft comment:
Possible lexicographical issue: "shared rules + falling cost = broad access" might be clearer as "shared rules + falling costs = broad access", if that aligns with the intended meaning. - 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 new marketing document explaining Jan's vision. The suggested change is extremely minor and doesn't affect meaning. Both singular and plural forms are acceptable here since "cost" can be used as a mass noun. This kind of nitpicky grammar suggestion on marketing copy doesn't add value. Perhaps maintaining perfect grammar is important for a company's public-facing documents? Maybe this level of polish matters for credibility? While polish matters, this particular suggestion is debatable since both forms are grammatically valid. The current form reads naturally and the change wouldn't meaningfully improve the document. Delete this comment as it's an extremely minor grammatical suggestion that doesn't improve clarity or correctness.
Workflow ID: wflow_9Ct3Pfo8H6Smi25P
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
|
Preview URL: https://1ea87d0e.docs-9ba.pages.dev |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Important
Looks good to me! 👍
Reviewed 2f85f21 in 1 minute and 46 seconds. Click for details.
- Reviewed
211lines of code in7files - 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. docs/src/components/FavoriteModels.tsx:94
- Draft comment:
Remove commented-out Button code if no longer needed or add a TODO comment explaining why it's temporarily disabled. - 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 about code that was intentionally commented out, likely as part of a UI change. The pattern repeats 3 times in the file, suggesting it's a deliberate choice to hide these buttons. Since this is a UI component and the buttons are purely UI elements, we should trust that the author knows what they're doing with the UI. The comment does raise a valid point about code cleanliness - commented out code can make files harder to maintain. However, this could be a temporary UI state or A/B test. Since this is a pure UI change and there are multiple instances of the same pattern, we should trust the author's intention and not micromanage UI decisions. Delete the comment since it's about a UI change and we should trust the author's UI decisions per the rules.
2. docs/src/components/Home/index.tsx:142
- Draft comment:
Consider providing a fallback default (e.g. 0) for discordWidget.presence_count to avoid potential runtime errors if the API data is undefined. - 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. docs/src/components/Navbar.tsx:11
- Draft comment:
Removal of the 'API Reference' menu item appears intentional; ensure corresponding documentation and navigation updates are in sync. - 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. docs/theme.config.tsx:110
- Draft comment:
Title template ordering has changed to prefix a sub-path-specific label. Confirm that this new format (e.g., 'Jan Desktop - Page Title') is intended. - Reason this comment was not posted:
Comment looked like it was already resolved.
5. docs/theme.config.tsx:161
- Draft comment:
Meta keywords are provided as an array; consider joining them into a comma-separated string to ensure proper meta tag formatting. - 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_ZZLbkjTpfO3zkui9
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
- Add betting-on-open-source.mdx and open-superintelligence.mdx - Update handbook index with new structure - Remove outdated handbook sections (growth, happy, history, money, talent, teams, users, why) - Update handbook _meta.json to reflect new structure
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Caution
Changes requested ❌
Reviewed 26ed125 in 2 minutes and 20 seconds. Click for details.
- Reviewed
345lines of code in13files - 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. docs/src/pages/handbook/_meta.json:2
- Draft comment:
New handbook page keys added. Ensure naming consistency with existing pages. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
2. docs/src/pages/handbook/index.mdx:19
- Draft comment:
Ensure removed handbook pages are redirected to avoid broken links. - 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 comment violates several rules. It starts with "Ensure that...", which is explicitly called out as not useful. It's asking the author to verify something rather than pointing out a clear issue. It's also speculative - we don't know if these pages were even public or if redirects are needed. The comment isn't about a specific code change that needs to be made. Maybe broken links could be a real user experience issue that needs to be addressed. The comment is trying to prevent a potential problem. While the intention is good, the comment doesn't meet our bar for being actionable and clearly correct. If redirects are needed, that should be caught by tests or deployment processes. Delete this comment as it's asking for verification rather than pointing out a specific required code change, and it starts with "Ensure that..." which violates our rules.
3. docs/theme.config.tsx:67
- Draft comment:
Verify the update from 'Jan Desktop & Mobile' to 'Jan Desktop' aligns with mobile functionality. - 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 if the update aligns with mobile functionality. This falls under asking the author to confirm their intention or to ensure the behavior is intended, which is against the rules.
4. docs/src/pages/handbook/betting-on-open-source.mdx:17
- Draft comment:
Typo: Consider revising 'invention' to 'inventions' in the phrase 'One of the most powerful invention is being steered by a handful of executives.' - Reason this comment was not posted:
Marked as duplicate.
5. docs/src/pages/handbook/open-superintelligence.mdx:37
- Draft comment:
Typographical error: In this line, "Superinteligence" appears to be misspelled. Consider correcting it to "Superintelligence". - Reason this comment was not posted:
Marked as duplicate.
Workflow ID: wflow_NUoTHwBZHTxykJY8
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Important
Looks good to me! 👍
Reviewed 3ec41e0 in 1 minute and 27 seconds. Click for details.
- Reviewed
268lines of code in9files - Skipped
0files when reviewing. - Skipped posting
8draft 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. docs/src/pages/docs/_meta.json:6
- Draft comment:
New 'index' meta added for 'Jan Overview'. Confirm its integration and desired ordering in navigation. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
2. docs/src/pages/docs/desktop/mcp-examples/browser/_meta.json:2
- Draft comment:
Removed 'href' for 'Browserbase'. Ensure routing or link-generation logic handles navigation without it. - Reason this comment was not posted:
Comment looked like it was already resolved.
3. docs/src/pages/docs/desktop/mcp-examples/data-analysis/_meta.json:2
- Draft comment:
Removed 'href' properties from entries. Verify that URL routing for 'E2B Code Sandbox' and 'Jupyter Notebooks' is managed elsewhere. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the author to verify that URL routing is managed elsewhere after removing 'href' properties. This is a request for confirmation, which violates the rule against asking the author to confirm their intention or ensure behavior. Therefore, this comment should be removed.
4. docs/src/pages/docs/desktop/mcp-examples/deepresearch/_meta.json:2
- Draft comment:
Removed 'href' from 'Octagon Deep Research'. Confirm that navigation logic no longer depends on explicit href 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% This is a documentation metadata file. The comment asks to verify/confirm something rather than pointing out a clear issue. It's speculative about potential navigation issues. We don't have evidence that navigation actually depends on href values. This seems like the kind of "verify that..." comment we should remove. Maybe there could be some navigation system that does depend on these href values, and removing them could break something? Without clear evidence that navigation depends on these values, this is exactly the kind of speculative "verify that..." comment we should avoid. If there was a navigation dependency, it would likely be caught by tests or builds. Delete this comment as it's asking for verification without evidence of an actual issue, and any real navigation problems would be caught by testing.
5. docs/src/pages/docs/desktop/mcp-examples/design/_meta.json:2
- Draft comment:
Canva meta now omits 'href'. Ensure that link resolution for design examples functions correctly without it. - Reason this comment was not posted:
Comment looked like it was already resolved.
6. docs/src/pages/docs/desktop/mcp-examples/productivity/_meta.json:2
- Draft comment:
Todoist and Linear entries now lack 'href'. Verify that these changes do not break navigation linking. - 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 violates several rules: 1) It starts with "Verify that..." which is explicitly discouraged 2) It's asking the author to double-check something rather than pointing out a definite issue 3) Without seeing the navigation system's code, we can't be certain this is even a problem 4) If this did break navigation, it would likely be caught by automated tests or builds Maybe removing hrefs could cause silent runtime errors that wouldn't be caught by the build system. Maybe this is a critical navigation issue. Even if there are potential issues, asking for verification isn't helpful - we should either point out a specific problem or trust the author's changes. Navigation issues would likely be caught in testing. Delete this comment as it violates our rules by asking for verification rather than pointing out a specific issue, and we don't have strong evidence that this change causes problems.
7. docs/src/pages/docs/desktop/mcp-examples/search/_meta.json:2
- Draft comment:
Exa and Serper Search meta now omit 'href'. Ensure that default routing or link generation covers these entries. - Reason this comment was not posted:
Comment looked like it was already resolved.
8. docs/src/pages/docs/desktop/remote-models/_meta.json:2
- Draft comment:
Remote models now only include 'title' without 'href'. Confirm that URL resolution for these models is handled by alternative logic. - 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 comment violates several rules. It's asking for confirmation ("Confirm that...") which is explicitly disallowed. It's speculative about URL resolution. If there was an actual issue with URL resolution, it would show up in tests or builds. The comment isn't suggesting any specific code changes. Maybe URL resolution is actually a critical issue that needs to be addressed before merging? If URL resolution was broken, it would be caught by tests/builds. Also, asking for confirmation is not actionable - either there's a specific issue to fix or there isn't. Delete this comment as it violates rules against asking for confirmation and making speculative comments without specific issues to fix.
Workflow ID: wflow_9c8OlgNA76kostWa
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Important
Looks good to me! 👍
Reviewed a6a2f0c in 1 minute and 39 seconds. Click for details.
- Reviewed
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. docs/src/pages/docs/desktop/_meta.json:8
- Draft comment:
Renamed key from 'desktop' to 'install'. Ensure that all references (e.g., routing or menu config) consistently use the new key. - 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 speculative comment asking for verification. It's asking the author to "ensure" something, which is explicitly against our rules. The comment is also about potential cross-file issues, which we're told to ignore. Build systems and tests would likely catch any broken references. The rename could potentially break navigation if not handled properly. Broken navigation can be a serious user experience issue. While navigation issues are important, we're explicitly told to ignore cross-file issues and to not make comments that ask for verification. We should trust the author and build system to handle this. This comment should be deleted as it violates multiple rules: it's asking for verification, it's speculative, and it concerns cross-file issues.
Workflow ID: wflow_dzpI77kKzysJzeoq
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
Describe Your Changes
This pull request introduces significant improvements to the documentation site's UI and structure, including a major redesign of the blog component, the addition of a new featured models section, updates to dependencies, and a refactor of the footer menu. The changes enhance user experience, visual consistency, and maintainability.
UI/UX Improvements:
Blogcomponent indocs/src/components/Blog/index.tsxto use a custom layout instead of the previousCardscomponent, improving readability, navigation, and category filtering. The new design features a timeline-style list with enhanced metadata display. [1] [2] [3]FavoriteModelssection indocs/src/components/FavoriteModels.tsx, showcasing popular AI models and app connectors with engaging visuals and animations usingframer-motion. This section also introduces a "Memory" feature preview.Dependency Updates:
docs/package.json, including addingframer-motionandclass-variance-authorityfor improved animations and styling, and bumpingreact-tweetto the latest version for better compatibility. [1] [2]Footer Refactor:
FooterMenucomponent indocs/src/components/FooterMenu/index.tsxto use a more structured and maintainable approach for menu items, replacing the previous hardcoded and commented-out configuration.Code Cleanup:
FooterMenu, streamlining the component and reducing bundle size.Fixes Issues
Self Checklist
Important
Enhance documentation site with UI/UX improvements, dependency updates, and code refactoring for better user experience and maintainability.
Blogcomponent inindex.tsxfor better readability and navigation.FavoriteModelssection inFavoriteModels.tsxwith animations usingframer-motion.package.jsonto includeframer-motionandclass-variance-authority.FooterMenuinindex.tsxfor a more structured approach.FooterMenu.This description was created by
for a6a2f0c. You can customize this summary. It will automatically update as commits are pushed.