-
Notifications
You must be signed in to change notification settings - Fork 80
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
docs(weave): Sidebar cleaning - Get Started contents #3810
base: master
Are you sure you want to change the base?
Conversation
WalkthroughThis pull request updates multiple documentation files. Changes include adding new sections (such as "Quickstart" and "Usage notes") and rephrasing, restructuring, and renaming titles and sections in several tutorial and guide documents. In addition, the sidebar configuration has been revised to reorganize navigation for these documents. No modifications were made to the underlying code functionality or control flow. Changes
Suggested reviewers
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (5)
docs/docs/quickstart.md (1)
34-47
: Clear step-by-step W&B account instructions!Breaking down the account creation and API key process into detailed steps improves clarity for new users.
There's a minor grammatical issue in line 36: "Next, create a Weights & Biases (W&B)." is missing the word "account". Also, line 43 should say "your W&B API key" instead of "you W&B API key".
-Next, create a Weights & Biases (W&B). +Next, create a Weights & Biases (W&B) account. -Once you've created your account, copy and set you W&B API key: +Once you've created your account, copy and set your W&B API key:docs/docs/tutorial-weave_models.md (2)
117-117
: Grammar issue in sentence about model versionsThere's a grammatical error in this sentence.
-After calling `.invoke`, you can view the trace in Weave. Now, model attributes are tracked along with model functions that have been decorated with `weave.op()`. You can see the model is also versioned (in the example, `v21`). Click on the model to see all of calls that have used that version of the model. +After calling `.invoke`, you can view the trace in Weave. Now, model attributes are tracked along with model functions that have been decorated with `weave.op()`. You can see the model is also versioned (in the example, `v21`). Click on the model to see all calls that have used that version of the model.🧰 Tools
🪛 LanguageTool
[grammar] ~117-~117: Consider using “all calls” or “all of the calls”.
Context: ...mple,v21
). Click on the model to see all of calls that have used that version of the mode...(ALL_MOST_SOME_OF_NOUN)
132-133
: Subject-verb agreement issueThe verb "builds" doesn't agree with the plural subject "examples".
-The following code examples builds on the example in [ Use `Model` to version an app](#use-model-to-version-an-app), and shows reuse of a `Model` version specified by `weave:///morgan/jurassic-park/object/ExtractDinos:ey4udBU2MU23heQFJenkVxLBX4bmDsFk7vsGcOWPjY4`. +The following code example builds on the example in [Use `Model` to version an app](#use-model-to-version-an-app), and shows reuse of a `Model` version specified by `weave:///morgan/jurassic-park/object/ExtractDinos:ey4udBU2MU23heQFJenkVxLBX4bmDsFk7vsGcOWPjY4`.Also note the removal of extra spaces inside the link text and after "of a".
🧰 Tools
🪛 LanguageTool
[grammar] ~132-~132: The verb form ‘builds’ does not seem to match the subject ‘examples’.
Context: ... your URI. The following code examples builds on the example in [ UseModel
to vers...(SUBJECT_VERB_AGREEMENT_PLURAL)
🪛 markdownlint-cli2 (0.17.2)
132-132: Spaces inside link text
null(MD039, no-space-in-links)
docs/docs/tutorial-tracing_2.md (2)
15-15
: Context on Nested Function Tracking
The paragraph aptly explains the need for monitoring nested functions in LLM-powered applications. Consider rephrasing "nested functions for LLMs calls" to "nested calls to LLMs" for clearer language.
17-17
: Clarification onweave.op()
The explanation regardingweave.op()
is informative. To improve further, consider adding a hyperlink or a reference to the detailed documentation onweave.op()
for users who want to explore its functionality.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
docs/docs/get-started.md
(1 hunks)docs/docs/guides/core-types/models.md
(1 hunks)docs/docs/quickstart.md
(3 hunks)docs/docs/tutorial-eval.md
(1 hunks)docs/docs/tutorial-tracing_2.md
(3 hunks)docs/docs/tutorial-weave_models.md
(4 hunks)docs/sidebars.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- docs/docs/get-started.md
- docs/docs/tutorial-eval.md
🧰 Additional context used
📓 Path-based instructions (2)
`**/*.{js,jsx,ts,tsx}`: Focus on architectural and logical i...
**/*.{js,jsx,ts,tsx}
: Focus on architectural and logical issues rather than style (assuming ESLint is in place).
Flag potential memory leaks and performance bottlenecks.
Check for proper error handling and async/await usage.
Avoid strict enforcement of try/catch blocks - accept Promise chains, early returns, and other clear error handling patterns. These are acceptable as long as they maintain clarity and predictability.
Ensure proper type usage in TypeScript files.
Look for security vulnerabilities in data handling.
Don't comment on formatting if prettier is configured.
Verify proper React hooks usage and component lifecycle.
Check for proper state management patterns.
docs/sidebars.ts
`**/*.{md,mdx}`: Focus on technical accuracy. Check for brok...
**/*.{md,mdx}
: Focus on technical accuracy.
Check for broken links.
Verify code examples are up-to-date.
Look for clarity and completeness.
Don't focus on grammar/spelling unless significant.
docs/docs/quickstart.md
docs/docs/guides/core-types/models.md
docs/docs/tutorial-weave_models.md
docs/docs/tutorial-tracing_2.md
🪛 LanguageTool
docs/docs/tutorial-weave_models.md
[grammar] ~117-~117: Consider using “all calls” or “all of the calls”.
Context: ...mple, v21
). Click on the model to see all of calls that have used that version of the mode...
(ALL_MOST_SOME_OF_NOUN)
[style] ~127-~127: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...he name of the Model
version that you want to reuse or export. A pop-up modal display...
(REP_WANT_TO_VB)
[grammar] ~132-~132: The verb form ‘builds’ does not seem to match the subject ‘examples’.
Context: ... your URI. The following code examples builds on the example in [ Use Model
to vers...
(SUBJECT_VERB_AGREEMENT_PLURAL)
🪛 markdownlint-cli2 (0.17.2)
docs/docs/tutorial-weave_models.md
132-132: Spaces inside link text
null
(MD039, no-space-in-links)
⏰ Context from checks skipped due to timeout of 90000ms (40)
- GitHub Check: Trace nox tests (3, 13, trace)
- GitHub Check: Trace nox tests (3, 12, trace)
- GitHub Check: Trace nox tests (3, 11, trace)
- GitHub Check: Trace nox tests (3, 10, trace)
- GitHub Check: Trace nox tests (3, 13, trace)
- GitHub Check: Trace nox tests (3, 12, trace)
- GitHub Check: Trace nox tests (3, 11, trace)
- GitHub Check: Trace nox tests (3, 10, trace)
- GitHub Check: Trace nox tests (3, 13, trace)
- GitHub Check: Trace nox tests (3, 12, trace)
- GitHub Check: Trace nox tests (3, 11, trace)
- GitHub Check: Trace nox tests (3, 10, trace)
- GitHub Check: Trace nox tests (3, 13, trace)
- GitHub Check: Trace nox tests (3, 12, trace)
- GitHub Check: Trace nox tests (3, 11, trace)
- GitHub Check: Trace nox tests (3, 10, trace)
- GitHub Check: Trace nox tests (3, 13, trace)
- GitHub Check: Trace nox tests (3, 12, trace)
- GitHub Check: Trace nox tests (3, 11, trace)
- GitHub Check: Trace nox tests (3, 10, trace)
- GitHub Check: Trace nox tests (3, 13, trace)
- GitHub Check: Trace nox tests (3, 12, trace)
- GitHub Check: Trace nox tests (3, 11, trace)
- GitHub Check: Trace nox tests (3, 10, trace)
- GitHub Check: Trace nox tests (3, 13, trace)
- GitHub Check: Trace nox tests (3, 12, trace)
- GitHub Check: Trace nox tests (3, 11, trace)
- GitHub Check: Trace nox tests (3, 10, trace)
- GitHub Check: Trace nox tests (3, 13, trace)
- GitHub Check: Trace nox tests (3, 12, trace)
- GitHub Check: Trace nox tests (3, 11, trace)
- GitHub Check: Trace nox tests (3, 10, trace)
- GitHub Check: Trace nox tests (3, 13, trace)
- GitHub Check: Trace nox tests (3, 12, trace)
- GitHub Check: Trace nox tests (3, 11, trace)
- GitHub Check: Trace nox tests (3, 10, trace)
- GitHub Check: Trace nox tests (3, 13, trace)
- GitHub Check: Trace nox tests (3, 12, trace)
- GitHub Check: Trace nox tests (3, 11, trace)
- GitHub Check: Trace nox tests (3, 10, trace)
🔇 Additional comments (18)
docs/docs/guides/core-types/models.md (1)
174-177
: Great addition of usage notes section!These notes provide valuable clarification about function naming, method tracking, and attribute visibility in Weave Models. The information will help users better understand how to structure their code.
docs/sidebars.ts (1)
14-37
: Good reorganization of the Get Started section!The restructuring with a dedicated "Overview" entry point and collapsible "Quickstart" section with multiple guides improves navigation and creates a more logical documentation flow. This hierarchical organization will help users find relevant guides more easily.
docs/docs/quickstart.md (2)
10-13
: Good addition of the Colab option!Adding this tip with the Colab button makes the documentation more interactive by offering users an alternative way to follow along with the quickstart.
62-64
: Important note about OpenAI API key is helpful!This alert box provides critical information that users need before attempting the example.
docs/docs/tutorial-weave_models.md (2)
4-7
: Improved title and introduction!The new title "Track app versions" better reflects the content's purpose, and the rewritten introduction provides clearer context about how this section builds on previous guides.
21-26
: Well-structured steps for creating a Model!The numbered list format makes these instructions much easier to follow.
docs/docs/tutorial-tracing_2.md (12)
4-4
: Clear Section Title Update
The updated title, "Track nested functions and metadata," effectively reflects the enhanced focus of this tutorial. Ensure that it remains consistent with other section titles in your documentation.
6-6
: Link Update for Quickstart Landing Page
The reference[Log a trace](/quickstart)
now points to the new quickstart landing page. Verify that this landing page is populated with appropriate content soon, to avoid confusing users.
8-8
: Introduction to Learning Objectives
The sentence "In this guide, you will learn how to:" clearly introduces the learning objectives. The message is straightforward and sets expectations well.
10-11
: Concise Learning Objectives
The bullet points for "Track nested function calls" and "Track metadata at call time" are succinct and clear. This format helps readers quickly grasp what they will learn.
13-13
: Section Header for Nested Function Calls
The header "## Track nested function calls" clearly defines the start of this section. Ensure it remains stylistically consistent with other headers in the documentation.
19-19
: Clarify Example Reference Link
The phrase "Building on the basic tracing example..." may be a bit ambiguous, especially in relation to the "[Log a trace]" tutorial mentioned earlier. Ensure the link text and intended destination are consistent or provide additional context to differentiate the examples.
80-80
: Instruction for Viewing Trace Data
The direction "To view the trace data for the inputs and outputs from the nested functions, as well as the automatically-logged OpenAI trace, run the code sample, and navigate to your Weave Traces tab." is clear and actionable. A minor tweak in punctuation for smoother readability might be considered.
145-145
: Metadata Tracking Section Header
The new header "## Track metadata" clearly marks the beginning of the metadata section. This update helps distinguish it from the previous nested function tracking content.
147-147
: Introduction to Metadata Tracking
The explanation "You can track metadata using theweave.attributes
context manager. To track metadata usingweave.attributes
, pass it a dictionary of metadata to track at call time." is clear and informative.
149-151
: Helpful Tip for Runtime Metadata
The tip encapsulated between the:::tip
markers is useful in emphasizing thatweave.attributes
is best suited for runtime metadata (e.g., user IDs and environment information). Ensure the markdown styling is consistent with your style guide.
152-152
: Link Reference to Weave Models
The sentence "To track system attributes, such as a System Prompt, use WeaveModel
s" provides a helpful pointer. Please verify that the linked page exists and that the URL is up to date.
155-155
: Metadata Logging Example Description
The explanation detailing how metadata is logged—building on the "Track nested function calls" example—is clear and informative. The inline reference enhances navigation and user clarity.
|
||
**Install weave** | ||
## 1. Prerequisites |
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.
Reviewers:
Would it make sense to copy all Prereqs content out into it's own page or something as "Step 0" for getting started with Weave? In other words, separate it from the tutorial for basic trace logging and maybe put it in the new Quickstart landing page (currently with a placeholder at https://github.com/wandb/weave/blob/9c6ed2410d129510dd9c76da366de5a51164d0b5/docs/docs/get-started.md) or something? Benefits: can link to standardized Weave prereqs (install weave
, sign up for W&B, get and set API key) from all tutorials in the docs (and other pages), without including any other functionality
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.
Since weave.init('project-name')
will prompt the user to login & copy their API key, i feel like we could move this content elsewhere / link to it and instead jump more quickly to the main content
Reviewer questions
|
@@ -0,0 +1,3 @@ | |||
# Quickstart | |||
|
|||
E plurbius unum... |
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.
empty page?
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.
Yeah, this was intentional. Wanted to make sure the team was good with the new structure before writing this content. I'll add now
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.
If you have any content you want added here, lmk. Idea here is that this new page is the "Quickstart" overview page - a high level overview of the path through the quickstart tutorials, what you'll learn, etc.
🚧 under construction
Description
As part of the epic to clean up the Weave docs sidebar, this PR focuses on everything under
👋 Getting Started
The PR makes these changes:
Questions for reviewers
#3810 (comment)
Testing