-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Add Notebook Research Agent (tools + streaming UI) #338
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: main
Are you sure you want to change the base?
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.
5 issues found across 12 files
Prompt for AI agents (all 5 issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="api/routers/agent.py">
<violation number="1" location="api/routers/agent.py:125">
P2: Exception message exposure: Using `str(e)` directly in client responses can leak sensitive internal details (file paths, configuration, etc.). Consider returning a generic error message to clients while logging the full details server-side.</violation>
</file>
<file name="frontend/src/lib/hooks/useAgent.ts">
<violation number="1" location="frontend/src/lib/hooks/useAgent.ts:6">
P2: Missing cleanup effect for unmount. If the component unmounts while streaming, `setState` calls will still fire on the unmounted component, causing memory leaks. Add a `useEffect` with cleanup that aborts any in-progress stream when the component unmounts.</violation>
<violation number="2" location="frontend/src/lib/hooks/useAgent.ts:53">
P1: The `stop()` function is non-functional because `abortControllerRef` is never initialized with `new AbortController()` and never connected to the stream. The abort controller should be created before streaming and its signal passed to the fetch call.</violation>
</file>
<file name="frontend/src/lib/api/agent.ts">
<violation number="1" location="frontend/src/lib/api/agent.ts:31">
P2: The `stream` method doesn't accept an `AbortSignal`, making it impossible to cancel ongoing requests. The consuming hook (`useAgent.ts`) has abort logic that won't work without this. Consider adding an optional `signal` parameter.</violation>
<violation number="2" location="frontend/src/lib/api/agent.ts:75">
P2: Any remaining data in the buffer is not processed after the stream ends. If the server sends a final SSE event without a trailing newline, that event will be lost. Consider processing the remaining buffer content after the loop.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
| logger.exception(e) | ||
| raise HTTPException( | ||
| status_code=500, | ||
| detail=f"Agent execution failed: {str(e)}" |
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.
P2: Exception message exposure: Using str(e) directly in client responses can leak sensitive internal details (file paths, configuration, etc.). Consider returning a generic error message to clients while logging the full details server-side.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At api/routers/agent.py, line 125:
<comment>Exception message exposure: Using `str(e)` directly in client responses can leak sensitive internal details (file paths, configuration, etc.). Consider returning a generic error message to clients while logging the full details server-side.</comment>
<file context>
@@ -0,0 +1,224 @@
+ logger.exception(e)
+ raise HTTPException(
+ status_code=500,
+ detail=f"Agent execution failed: {str(e)}"
+ )
+
</file context>
| * Manages agent state and interactions | ||
| */ | ||
|
|
||
| import { useState, useCallback, useRef } from 'react' |
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.
P2: Missing cleanup effect for unmount. If the component unmounts while streaming, setState calls will still fire on the unmounted component, causing memory leaks. Add a useEffect with cleanup that aborts any in-progress stream when the component unmounts.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/lib/hooks/useAgent.ts, line 6:
<comment>Missing cleanup effect for unmount. If the component unmounts while streaming, `setState` calls will still fire on the unmounted component, causing memory leaks. Add a `useEffect` with cleanup that aborts any in-progress stream when the component unmounts.</comment>
<file context>
@@ -0,0 +1,196 @@
+ * Manages agent state and interactions
+ */
+
+import { useState, useCallback, useRef } from 'react'
+import { agentApi } from '../api/agent'
+import type {
</file context>
|
|
||
| // Refs | ||
| const threadIdRef = useRef<string>(`agent-${Date.now()}`) | ||
| const abortControllerRef = useRef<AbortController | null>(null) |
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.
P1: The stop() function is non-functional because abortControllerRef is never initialized with new AbortController() and never connected to the stream. The abort controller should be created before streaming and its signal passed to the fetch call.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/lib/hooks/useAgent.ts, line 53:
<comment>The `stop()` function is non-functional because `abortControllerRef` is never initialized with `new AbortController()` and never connected to the stream. The abort controller should be created before streaming and its signal passed to the fetch call.</comment>
<file context>
@@ -0,0 +1,196 @@
+
+ // Refs
+ const threadIdRef = useRef<string>(`agent-${Date.now()}`)
+ const abortControllerRef = useRef<AbortController | null>(null)
+
+ // Generate unique ID
</file context>
| try { | ||
| while (true) { | ||
| const { done, value } = await reader.read() | ||
| if (done) break |
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.
P2: Any remaining data in the buffer is not processed after the stream ends. If the server sends a final SSE event without a trailing newline, that event will be lost. Consider processing the remaining buffer content after the loop.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/lib/api/agent.ts, line 75:
<comment>Any remaining data in the buffer is not processed after the stream ends. If the server sends a final SSE event without a trailing newline, that event will be lost. Consider processing the remaining buffer content after the loop.</comment>
<file context>
@@ -0,0 +1,118 @@
+ try {
+ while (true) {
+ const { done, value } = await reader.read()
+ if (done) break
+
+ buffer += decoder.decode(value, { stream: true })
</file context>
| * Stream agent execution | ||
| * Returns an async generator that yields stream events | ||
| */ | ||
| async *stream(request: AgentExecuteRequest): AsyncGenerator<AgentStreamEvent> { |
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.
P2: The stream method doesn't accept an AbortSignal, making it impossible to cancel ongoing requests. The consuming hook (useAgent.ts) has abort logic that won't work without this. Consider adding an optional signal parameter.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/lib/api/agent.ts, line 31:
<comment>The `stream` method doesn't accept an `AbortSignal`, making it impossible to cancel ongoing requests. The consuming hook (`useAgent.ts`) has abort logic that won't work without this. Consider adding an optional `signal` parameter.</comment>
<file context>
@@ -0,0 +1,118 @@
+ * Stream agent execution
+ * Returns an async generator that yields stream events
+ */
+ async *stream(request: AgentExecuteRequest): AsyncGenerator<AgentStreamEvent> {
+ // Get API URL dynamically
+ const apiUrl = await getApiUrl()
</file context>
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.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="docs/features/cli-tool.md">
<violation number="1" location="docs/features/cli-tool.md:1">
P1: This documentation appears to be misplaced or accidentally included. It describes 'ACM Scholar CLI' - a completely different tool with external pip packages and GitHub repositories - which doesn't exist in this repository and is unrelated to the PR's Notebook Research Agent feature. Consider removing this file or replacing it with documentation for the actual agent feature being added.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| @@ -0,0 +1,118 @@ | |||
| # 🖥️ ACM Scholar CLI | |||
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.
P1: This documentation appears to be misplaced or accidentally included. It describes 'ACM Scholar CLI' - a completely different tool with external pip packages and GitHub repositories - which doesn't exist in this repository and is unrelated to the PR's Notebook Research Agent feature. Consider removing this file or replacing it with documentation for the actual agent feature being added.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/features/cli-tool.md, line 1:
<comment>This documentation appears to be misplaced or accidentally included. It describes 'ACM Scholar CLI' - a completely different tool with external pip packages and GitHub repositories - which doesn't exist in this repository and is unrelated to the PR's Notebook Research Agent feature. Consider removing this file or replacing it with documentation for the actual agent feature being added.</comment>
<file context>
@@ -0,0 +1,118 @@
+# 🖥️ ACM Scholar CLI
+
+A powerful command-line tool for academic research, powered by Gemini API.
</file context>
|
Very nice feature! Hope the admin accept the PR soon |
|
@hongping-zh did you get the video uploaded? |
Summary
This PR adds an initial prototype of a notebook-scoped research agent (ReAct-style) to Open Notebook.
It focuses on:
This should align well with a future “deep research” feature since the UI shows intermediate actions/results and the agent can be extended with additional tools/workflows.
Demo
Video (1–2 min): TBD (uploading)
Key Changes
Backend
/api/agent/*endpoints (execute + SSE stream + tools + models)Frontend
frontend/src/components/agent/*: Agent UI panel (streaming steps)frontend/src/app/(dashboard)/notebooks/components/ChatColumn.tsx: Chat/Agent tab switchHow to Run (dev)
python run_api.pycd frontend && npm run devdeepseek-chat)Notes