-
Notifications
You must be signed in to change notification settings - Fork 2k
feat(max): deep research template system #39104
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
base: master
Are you sure you want to change the base?
Conversation
feat: deep research templates chore: mypy
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.
Additional Comments (1)
-
ee/hogai/graph/deep_research/onboarding/nodes.py
, line 77 (link)logic: Resetting current_run_notebooks to empty list may conflict with the skip_onboarding flow that expects notebooks to persist
10 files reviewed, 5 comments
assistant_initial_state = None | ||
if initial_state is not None: | ||
if isinstance(initial_state, (AssistantState | PartialAssistantState)): | ||
assistant_initial_state = initial_state |
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.
style: This code duplicates the type checking logic from the ASSISTANT mode. Consider extracting this pattern into a helper method to avoid repetition.
Prompt To Fix With AI
This is a comment left during a code review.
Path: ee/hogai/assistant/assistant.py
Line: 55:58
Comment:
**style:** This code duplicates the type checking logic from the ASSISTANT mode. Consider extracting this pattern into a helper method to avoid repetition.
How can I resolve this? If you propose a fix, please make it concise.
class DeepResearchOnboardingNode(DeepResearchNode): | ||
@property | ||
def node_name(self) -> MaxNodeName: | ||
return DeepResearchNodeName.NOTEBOOK_PLANNING |
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.
logic: The node_name returns NOTEBOOK_PLANNING but the class is DeepResearchOnboardingNode - this seems inconsistent and could be confusing
Prompt To Fix With AI
This is a comment left during a code review.
Path: ee/hogai/graph/deep_research/onboarding/nodes.py
Line: 20:20
Comment:
**logic:** The node_name returns NOTEBOOK_PLANNING but the class is DeepResearchOnboardingNode - this seems inconsistent and could be confusing
How can I resolve this? If you propose a fix, please make it concise.
if not isinstance(last_message, HumanMessage): | ||
raise ValueError("Last message is not a human message.") | ||
|
||
human_content = last_message.content if last_message else "" |
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.
logic: Redundant condition - last_message
is guaranteed to exist due to the check on line 42
human_content = last_message.content if last_message else "" | |
human_content = last_message.content |
Prompt To Fix With AI
This is a comment left during a code review.
Path: ee/hogai/graph/deep_research/notebook/nodes.py
Line: 45:45
Comment:
**logic:** Redundant condition - `last_message` is guaranteed to exist due to the check on line 42
```suggestion
human_content = last_message.content
```
How can I resolve this? If you propose a fix, please make it concise.
if not ( | ||
state.template_notebook_short_id and not state.template_markdown and not state.has_emitted_template_loaded | ||
): |
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.
style: Complex nested boolean logic makes this condition hard to understand. Consider extracting to a helper method with descriptive name like should_retrieve_template_from_db
.
Prompt To Fix With AI
This is a comment left during a code review.
Path: ee/hogai/graph/deep_research/notebook/nodes.py
Line: 107:109
Comment:
**style:** Complex nested boolean logic makes this condition hard to understand. Consider extracting to a helper method with descriptive name like `should_retrieve_template_from_db`.
How can I resolve this? If you propose a fix, please make it concise.
Problem
For Max Deep Research, one possible pain point during the onboarding phase is for the user to go through receiving a long list of questions from Max that needs to be answered. This can feel tiresome and make the experience not particularly great.
Idea: Have a list of curated + custom templates (notebooks) that the user can compile in their own time and then load as onboarding step when starting Deep Research.
This PR builds on top of the foundational piece for notebooks tags (#39101) and add backend support for Max and Notebooks API.
How did you test this code?
deep_research_templates_e2e.mp4