fix(studio): fix context pack generation, duplicate import UX, and CLI chat errors#223
Merged
Conversation
…LI chat errors - 支持内联标题/摘要，以绕过 PaperInputRouter 获取 studio 论文 ID - 使上下文包生成 SSE 流期间的数据库持久化操作不再致命 - 从库中导入重复论文时显示错误消息 - 将左侧文件面板宽度缩小至约 18%（默认宽度）；移除默认模板文件 - 为 Claude CLI 添加 --verbose 标志，以兼容 stream-json 输出 Related to jerry609#222
|
@CJBshuosi is attempting to deploy a commit to the Jerry's projects Team on Vercel. A member of the Team first needs to authorize it. |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly refines the studio experience by improving the robustness of context pack generation, making the paper import process more user-friendly, and enhancing the integration with the Claude CLI for more reliable and detailed chat interactions. These changes collectively aim to provide a smoother and more informative workflow for users within the studio environment. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
这个 PR 包含了一系列对 Studio 功能的重要修复和改进，做得很好。主要亮点包括：
- 使上下文包（context pack）生成过程中的数据库持久化操作变为非致命性，提高了系统的健壮性。
- 改进了从库中导入重复论文时的用户体验，增加了明确的错误提示。
- 重构了 Studio 的主界面，引入了画廊（Gallery）视图和工作区（Workspace）视图，大大提升了可用性。
- 切换到 Claude CLI 的
stream-json输出格式，并重写了前端的事件处理逻辑，使得与 CLI 的交互更加可靠和结构化。
代码整体质量很高。我发现的主要问题是几处静默处理异常的地方，这可能会在生产环境中隐藏问题。请查看我的具体评论。
Note: Security Review did not run due to the size of the PR.
Comment on lines
+130
to
+131
| except Exception: | ||
| pass |
There was a problem hiding this comment.
这里的 except Exception: pass 会静默地忽略所有保存阶段性结果时发生的错误。这可能会隐藏数据库或存储层的问题，增加调试难度。建议至少记录一条警告日志，类似于此文件中对其他数据库操作的处理方式。
except Exception as exc:
Logger.warning(
f"[M2] store_save_stage_result_failed pack_id={pack_id} stage_name={stage_name} error={exc}",
file=LogFiles.ERROR,
)
Comment on lines
+209
to
+210
| except Exception: | ||
| pass |
There was a problem hiding this comment.
与上一个评论类似，这个 except Exception: pass 块在更新状态为 "failed" 时静默地忽略了错误。如果此操作失败，数据库中的状态可能永远停留在 "running"，并且前端可能无法得知生成已失败。这里也应该记录一条警告日志。
except Exception as exc:
Logger.warning(
f"[M2] store_update_status_on_error_failed pack_id={pack_id} error={exc}",
file=LogFiles.ERROR,
)
Comment on lines
+102
to
+104
| } catch (e) { | ||
| console.error("Failed to delete project files:", e) | ||
| } |
There was a problem hiding this comment.
在删除项目文件时，catch 块仅将错误输出到控制台。如果删除文件的 API 调用失败，用户不会收到任何通知，但该论文条目仍然会从 UI 状态中移除。这可能导致服务器上出现用户不知情的孤立文件。
建议在此处向用户显示一个错误通知（例如 toast message），并考虑在文件删除失败时，不从本地状态中删除该论文条目，以确保数据一致性。
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (9)
web/src/lib/store/project-context.ts (1)
35-40:⚠️ Potential issue | 🟡 Minor
updateFilemay create a malformed entry if the file doesn't exist.If
state.files[name]isundefined, the spread produces{ content }which lacks the requirednameandlanguageproperties ofVirtualFile. This could cause runtime errors when components attempt to access these properties.🛡️ Proposed fix to guard against missing file
- updateFile: (name, content) => set((state) => ({ - files: { - ...state.files, - [name]: { ...state.files[name], content } - } - })), + updateFile: (name, content) => set((state) => { + if (!state.files[name]) return state + return { + files: { + ...state.files, + [name]: { ...state.files[name], content } + } + } + }),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/lib/store/project-context.ts` around lines 35 - 40, The updateFile updater may produce a malformed VirtualFile when state.files[name] is undefined; modify updateFile to first check whether state.files[name] exists and either (a) merge content into the existing file ({ ...state.files[name], content }) or (b) create a proper VirtualFile with required properties (include name and a sensible language fallback, e.g., state.files[name]?.language || 'text') before setting it; reference the updateFile function and the VirtualFile shape when implementing this guard so callers never get an entry missing name or language.src/paperbot/api/routes/runbook.py (1)
214-267:⚠️ Potential issue | 🟠 MajorAdd tests for the allowlist mutation behavior changes.
This endpoint’s behavior changed materially (default toggle + new path normalization/denial flow), but no test updates are included here.
As per coding guidelines,
{src,web/src}/**/*.{py,ts,tsx}: If behavior changes, add or update tests.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/paperbot/api/routes/runbook.py` around lines 214 - 267, Add unit tests for the changed behavior around add_allowed_dir: cover when _runtime_allowlist_mutation_enabled() returns false (should 403), the default-true case, normalization/expansion of "~" and "~/..." to Path.home(), rejection of entries in _DENIED_PATHS and exact home directory (403s), rejection of invalid or non-existent paths (400), and a successful add that calls _save_runtime_allowed_dir and returns "ok", the resolved "directory" string and the result of _allowed_workdir_prefixes(); use mocking for Path.exists()/is_dir() and for _save_runtime_allowed_dir/_allowed_workdir_prefixes to assert calls and responses. Ensure tests reference add_allowed_dir, _runtime_allowlist_mutation_enabled, _DENIED_PATHS, _save_runtime_allowed_dir, and _allowed_workdir_prefixes.web/src/hooks/useContextPackGeneration.ts (1)
64-97:⚠️ Potential issue | 🟠 MajorAdd hook tests for payload construction and error-message precedence.
Both request-shaping and error extraction logic changed and are easy to regress without focused tests.
As per coding guidelines
{src,web/src}/**/*.{py,ts,tsx}:If behavior changes, add or update tests.Also applies to: 140-142
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/hooks/useContextPackGeneration.ts` around lines 64 - 97, Add unit tests for the useContextPackGeneration hook focusing on the generate function: one test should assert the POST payload sent to "/api/research/repro/context" contains paper_id, user_id (defaulting to "default" when absent), depth (defaulting to "standard"), and optional title/abstract only when provided; another test should exercise error handling to ensure error-message precedence (e.g., prefer a parsed error message from the response body over a generic fetch/network error) so regressions in request-shaping and error extraction are caught.web/src/lib/store/studio-store.ts (1)
188-218:⚠️ Potential issue | 🟠 MajorCreating a new paper skips cache-save for the currently selected paper.
selectPapersaves per-paper state before switching, butaddPaperswitches selection directly and clears scoped state. This can drop context pack/progress for the previously selected paper.🛠️ Suggested fix
addPaper: (paper) => { @@ set(state => { + const newCache = { ...state._paperCache } + if (state.selectedPaperId) { + newCache[state.selectedPaperId] = { + contextPack: state.contextPack, + contextPackLoading: state.contextPackLoading, + contextPackError: state.contextPackError, + generationProgress: state.generationProgress, + liveObservations: state.liveObservations, + activeTaskId: state.activeTaskId, + } + } const newPapers = [...state.papers, newPaper] savePapersToStorage(newPapers) return { papers: newPapers, selectedPaperId: id, + _paperCache: newCache, // Sync paperDraft with new paper paperDraft: {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/lib/store/studio-store.ts` around lines 188 - 218, addPaper currently switches selectedPaperId and clears per-paper scoped state without persisting the current selected paper's transient data, causing loss of contextPack, generationProgress, liveObservations, lastGenCodeResult, workspaceSnapshotId, and related flags; update addPaper to persist the current paper's per-paper state before changing selection by reusing the same save logic used in selectPaper (or call a extracted helper the two share) so that the outgoing paper's contextPack, contextPackLoading, contextPackError, generationProgress, liveObservations, lastGenCodeResult, and workspaceSnapshotId are stored (e.g., via the savePapersToStorage/ per-paper save flow) before setting selectedPaperId and resetting paperDraft and scoped fields for the new paper.web/src/components/studio/ReproductionLog.tsx (1)
315-385:⚠️ Potential issue | 🟠 MajorAdd tests for the new CLI-event → action mapping paths.
The chat timeline behavior now depends on multiple event subtypes (
text,tool_use,tool_result,thinking,keepalive, fallbackdelta), so this should be covered with focused component/store integration tests.As per coding guidelines
{src,web/src}/**/*.{py,ts,tsx}:If behavior changes, add or update tests.Also applies to: 442-472
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/studio/ReproductionLog.tsx` around lines 315 - 385, Add integration tests for the new CLI-event → action mapping in ReproductionLog.tsx: write tests that feed mocked SSE events (simulate readSSE stream) with cli_event values "text", "tool_use", "tool_result", "thinking", a keepalive payload, legacy message, and fallback "delta", then assert that addAction/appendToLastAction are called with the correct action types/contents and metadata, that updateTaskStatus/setStatus are invoked for "result" and "error" events, and that keepalive is ignored; use the component/store test harness to mount the component (or call the handler) and verify action order, merging of streaming text into the last text action, attaching tool_result to the preceding function_call, and proper error handling via setLastError.web/src/components/studio/NewPaperModal.tsx (1)
95-152:⚠️ Potential issue | 🟠 MajorPlease add tests for manual and library duplicate-import behavior.
These paths now have non-trivial branching (manual title dedupe, multi-select skip logic, singular/plural error messaging) and should be covered to prevent regressions.
As per coding guidelines
{src,web/src}/**/*.{py,ts,tsx}:If behavior changes, add or update tests.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/studio/NewPaperModal.tsx` around lines 95 - 152, Add unit/integration tests covering duplicate detection and messaging for both manual and library imports: for handleManualSubmit, test that submitting a title that matches an existing paper (case/whitespace-insensitive) sets the error "A paper with this title already exists in the studio." and does not call addPaper or resetAndClose; also test successful submission calls addPaper with trimmed fields and calls resetAndClose. For handleImportSelected, test multi-select behavior where some selected library papers are already in the studio by id or title (case/whitespace-insensitive): ensure duplicates are skipped, non-duplicates are added (addPaper called for each), existingTitles is updated, and when all selected are duplicates the correct singular vs plural error messages are set ("This paper already exists..." vs "The selected papers already exist...") and resetAndClose is not called. Reference handleManualSubmit, handleImportSelected, addPaper, resetAndClose, selectedPaperIds, libraryPapers, and papers when writing tests.src/paperbot/api/routes/repro_context.py (1)
42-50:⚠️ Potential issue | 🟠 MajorPlease add integration tests for new inline and persistence-tolerant flows.
The route now has new request shape + fallback behavior under DB persistence failures; both should be covered explicitly.
As per coding guidelines
{src,web/src}/**/*.{py,ts,tsx}:If behavior changes, add or update tests.Also applies to: 166-176, 231-245
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/paperbot/api/routes/repro_context.py` around lines 42 - 50, The new GenerateContextPackRequest shape (fields title, abstract and optional persistence behavior) and the route's fallback on DB persistence failures lack integration tests; add tests that exercise (1) the inline-paper flow by POSTing GenerateContextPackRequest with title and abstract to the repro_context route and asserting the input-router is bypassed and expected context pack is returned, and (2) the persistence-tolerant flow by simulating DB persistence failures (mock the persistence layer used by the route) and asserting the route still returns a valid context pack and does not crash or block; update or add tests under the matching patterns for integration tests ({src,web/src}/**/*.{py,ts,tsx}), referencing GenerateContextPackRequest and the repro_context route handler to locate the code, and include cases for the other mentioned ranges (lines around 166-176 and 231-245) that changed behavior.web/src/app/studio/page.tsx (2)
108-111:⚠️ Potential issue | 🟠 MajorInline metadata generation path is still blocked by
paper_idrequirement.Lines 108-111 allow creating a paper from inline
title/abstract, but Lines 133-141 still hard-fail generation whenpaper_idis absent. This breaks thetitle/abstract + generate=trueflow.Please route
generate()through the created/selected studio paper id whenpaper_idis missing.Also applies to: 133-141
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/app/studio/page.tsx` around lines 108 - 111, The generate flow fails when users submit inline title+abstract because generate() still requires paper_id; update the logic so after creating or selecting a paper via addPaper({ title, abstract }) you pass the resulting/selected paper's id into generate() when paper_id is missing. Concretely, ensure the branch that calls addPaper stores or awaits the created paper id (from addPaper or its callback) and then calls generate(paper_id) (or uses that id where generate() is invoked), and modify the block around the existing paper_id checks (the code that currently hard-fails when paper_id is absent in the generate path) to use the newly created paper id instead of aborting.
85-159:⚠️ Potential issue | 🟠 MajorBehavior changed substantially, but no test updates are included.
This file adds new param-driven branching and a new gallery/workspace render flow. Please add or update tests for:
- query param ingestion (
paper_id,title,abstract,generate,context_pack_id)- gallery vs workspace rendering based on
selectedPaperId- generate/error behavior for missing ids
As per coding guidelines,
{src,web/src}/**/*.{py,ts,tsx}: If behavior changes, add or update tests.Also applies to: 161-234
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/app/studio/page.tsx` around lines 85 - 159, The new useEffect in page.tsx introduces query-param-driven behavior (reading paper_id, title, abstract, generate, context_pack_id), context pack loading (setContextPack, setContextPackError, setContextPackLoading), paper creation/selection (addPaper, selectPaper) and generate triggering (generate) plus URL cleaning via router.replace, but no tests were added; add/update unit/integration tests to cover: 1) ingestion of query params to create or select papers (paper_id, title, abstract) and that hasProcessedParams prevents reprocessing, 2) context pack fetch flow including success (normalizes via normalizePack and calls setContextPack) and failure (sets contextPackError and loading toggles), 3) generate path when generate=true both with and without paper_id to ensure generate({ paperId }) is called and missing id sets contextPackError, and 4) rendering differences between gallery and workspace based on selectedPaperId; target tests at the component level for the Studio page (page.tsx) mocking fetch/responses and router.searchParams, and assert router.replace("/studio") is called when shouldCleanUrl is true.
🧹 Nitpick comments (2)
web/src/components/studio/ReproductionLog.tsx (1)
341-351:tool_resultcurrently creates a new action instead of attaching to the last tool call.The code comment says “attach result to the most recent function_call action”, but the implementation appends a separate
function_callaction. Consider updating the previous tool-use action to keep one coherent entry per tool invocation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/studio/ReproductionLog.tsx` around lines 341 - 351, The "tool_result" branch is incorrectly adding a new function_call action instead of updating the most recent tool invocation; change the logic so that when cliEvent === "tool_result" you locate the most recent action for the given taskId with type "function_call" (use whatever action list/storage is used by addAction/getActions) and update that action's metadata.result and content rather than calling addAction again; adjust usage of lastActionIsText as needed to reflect that the existing action was updated; use identifiers like addAction, lastActionIsText, taskId, and the "function_call" action type to find and modify the correct entry.web/src/components/studio/PaperIcon.tsx (1)
72-74: Use a more descriptive accessible name for the SVG.Using only the 2-letter abbreviation as
aria-labelis ambiguous. Consider exposing the full paper title for better screen-reader output.♿ Suggested tweak
- role="img" - aria-label={abbr} + role="img" + aria-label={`Paper icon for ${title}`} > + <title>{`Paper icon for ${title}`}</title>Also applies to: 87-99
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/studio/PaperIcon.tsx` around lines 72 - 74, The SVG uses an ambiguous two-letter `abbr` for aria-label; in the PaperIcon component update the SVG `aria-label` to the full paper title (e.g., use the `title`/`paperTitle` prop if available) and fall back to `abbr` only if the full title is missing, and apply the same change to the other SVG instance referenced around the 87-99 area; modify the PaperIcon props or usage so `title`/`paperTitle` is passed through and used for accessibility while keeping `abbr` as a fallback.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/paperbot/api/routes/repro_context.py`:
- Around line 120-131: The try/except around the asyncio.to_thread call to
_store.save_stage_result currently swallows all exceptions (except Exception:
pass); change it to catch Exception as e and log the failure (including pack_id
and stage_name) using the module logger (e.g., logger.exception or logger.error
with exception info) so persistence failures in the SSE path are recorded but do
not break the stream; apply the same change to the identical block that appears
around the other save_stage_result call (the one referenced at lines 207-210).
- Around line 169-176: Summary: Inline mode is skipped when abstract == ""
because the condition requires both title and abstract to be truthy; change the
check to allow empty abstracts. Update the conditional that constructs
RawPaperData (look for request.title, request.abstract, inline_raw) so it
requires a non-empty title but treats abstract as present when it is not None
(e.g., use "request.title and request.abstract is not None" or equivalent), then
proceed to build RawPaperData with paper_id, title, abstract,
source_adapter="inline" so studio papers with empty abstracts still use inline
bypass.
In `@src/paperbot/api/routes/runbook.py`:
- Line 215: The runtime allowlist-mutation flag currently defaults to enabled by
returning os.getenv("PAPERBOT_RUNBOOK_ALLOWLIST_MUTATION", "true").lower() ==
"true"; change the default to disabled by using "false" as the fallback so the
environment opt-in is required (i.e., set default to "false" for the
PAPERBOT_RUNBOOK_ALLOWLIST_MUTATION getenv call), and update any related tests
or documentation that assume the default was true to reflect the new secure
default.
- Around line 246-257: The current exact-match check against _DENIED_PATHS and
the home-directory check allow descendants (e.g., /etc/ssh) through; change them
to reject any path that is the same as or a descendant of a denied path or of
the home dir by resolving each denied entry and using Path.is_relative_to (or
equivalent) against resolved (i.e., replace "if str(resolved) in _DENIED_PATHS"
with a loop/any that compares resolved == denied_resolved or
resolved.is_relative_to(denied_resolved) for each denied entry in _DENIED_PATHS,
and update the home check to reject when resolved == home_resolved or
resolved.is_relative_to(home_resolved)).
In `@src/paperbot/api/routes/studio_chat.py`:
- Around line 250-327: The new streaming contract for Claude CLI output
(cli_event variants, line-buffer parsing, trailing-buffer parse, and keepalive
emissions) isn't covered by tests; add route-level tests that exercise
_parse_cli_content_blocks and _parse_cli_event behavior (including "assistant"
content blocks, "tool_result", "result", ignored "system"), validate StreamEvent
objects produced (cli_event values like
"text","thinking","tool_use","tool_result","done"), ensure _truncate is applied
to long tool content, and add tests that simulate line-buffered and
trailing-buffer NDJSON inputs plus keepalive/heartbeat emissions to assert
correct event emission order and resilience to partial lines.
- Around line 384-443: The loop reads only process.stdout which can deadlock if
process.stderr (created via stderr=asyncio.subprocess.PIPE) fills; modify the
streaming logic around the subprocess `process` to concurrently drain stderr:
spawn an asyncio task (e.g., `stderr_task` created with asyncio.create_task)
that continuously reads from `process.stderr` into a buffer or discards it while
stdout is being streamed, ensure the stderr task is awaited (or cancelled and
awaited) after the stdout loop completes and before checking
`process.returncode`, and then use the collected stderr buffer when building
`error_msg`; update references in the code around the subprocess creation and
the existing `await process.wait()` / `if process.returncode != 0` block to
coordinate with the new `stderr_task`.
In `@web/src/app/studio/page.tsx`:
- Around line 177-185: The icon-only back button (Button with onClick={() =>
selectPaper(null)} and child ArrowLeft) relies only on title which isn't a
reliable accessible name; add an explicit accessible name by adding an
aria-label (e.g., aria-label="Back to gallery") to the Button element so
assistive technologies can announce it, keeping the existing title if desired
for hover tooltips.
In `@web/src/components/studio/NewPaperModal.tsx`:
- Around line 118-130: existingIds is built from StudioPaper.id but
selectedPaperIds are library IDs, so the ID-based dedupe in the NewPaperModal
import loop never matches; update the StudioPaper model to persist the
source/library ID (e.g., add a sourceId or libraryId field) and change the
dedupe check to compare selectedPaperIds against that persisted field (replace
existingIds construction to use studioPapers.map(p => p.sourceId) and ensure
code that creates StudioPaper sets sourceId from the library paper), or if you
cannot persist yet, remove the ID branch and rely on title dedupe only until you
add the sourceId field.
In `@web/src/components/studio/PaperGallery.tsx`:
- Around line 95-104: The fetch call triggered when deleting a paper in
PaperGallery.tsx currently ignores HTTP error responses; update the delete flow
that uses paperToDelete and the "/api/runbook/delete" POST so it checks the
fetch response (res.ok) and handles non-ok responses by logging or throwing a
descriptive error (including status and body text) inside the try/catch, and
propagate or surface the failure to the UI instead of treating all responses as
success.
In `@web/src/hooks/useContextPackGeneration.ts`:
- Around line 87-88: The truthy checks in useContextPackGeneration (the payload
building that sets payload.title and payload.abstract from params.title and
params.abstract) drop intentionally empty strings like ""; change the checks to
test for undefined (e.g., params.abstract !== undefined) or check hasOwnProperty
before assigning so empty but valid values are preserved and still sent to the
server; update both the title and abstract assignments in the payload
construction to use these explicit-presence checks.
In `@web/src/lib/store/studio-store.ts`:
- Around line 249-257: When removing a paper (the conditional block that checks
state.selectedPaperId === paperId and resets paperDraft, lastGenCodeResult,
contextPack, etc.), also clear the per-paper active task by setting activeTaskId
to null so stale task context isn't retained; update the cleanup object inside
that conditional (the same place where paperDraft, lastGenCodeResult,
contextPackLoading, contextPackError, generationProgress, and liveObservations
are cleared) to include activeTaskId: null.
- Around line 104-112: Add unit tests that verify the PerPaperCache round-trip
and the streaming-append behavior of appendToLastAction: 1) create a
PerPaperCache instance (exercise keys contextPack, contextPackLoading,
contextPackError, generationProgress, liveObservations, activeTaskId), save it
into the store, switch papers (simulate paper-id change) and assert the cache
for the original paper is preserved and can be restored exactly; 2) test
appendToLastAction by dispatching/committing an action that creates an initial
action item, then call appendToLastAction multiple times with incremental chunks
and assert the last action’s content and metadata are updated incrementally
(including edge cases: no existing action, empty chunk), and that
generationProgress/liveObservations updates behave like streaming appends; use
the existing studio-store methods and state selectors to locate PerPaperCache
and appendToLastAction in the store for assertions.
---
Outside diff comments:
In `@src/paperbot/api/routes/repro_context.py`:
- Around line 42-50: The new GenerateContextPackRequest shape (fields title,
abstract and optional persistence behavior) and the route's fallback on DB
persistence failures lack integration tests; add tests that exercise (1) the
inline-paper flow by POSTing GenerateContextPackRequest with title and abstract
to the repro_context route and asserting the input-router is bypassed and
expected context pack is returned, and (2) the persistence-tolerant flow by
simulating DB persistence failures (mock the persistence layer used by the
route) and asserting the route still returns a valid context pack and does not
crash or block; update or add tests under the matching patterns for integration
tests ({src,web/src}/**/*.{py,ts,tsx}), referencing GenerateContextPackRequest
and the repro_context route handler to locate the code, and include cases for
the other mentioned ranges (lines around 166-176 and 231-245) that changed
behavior.
In `@src/paperbot/api/routes/runbook.py`:
- Around line 214-267: Add unit tests for the changed behavior around
add_allowed_dir: cover when _runtime_allowlist_mutation_enabled() returns false
(should 403), the default-true case, normalization/expansion of "~" and "~/..."
to Path.home(), rejection of entries in _DENIED_PATHS and exact home directory
(403s), rejection of invalid or non-existent paths (400), and a successful add
that calls _save_runtime_allowed_dir and returns "ok", the resolved "directory"
string and the result of _allowed_workdir_prefixes(); use mocking for
Path.exists()/is_dir() and for
_save_runtime_allowed_dir/_allowed_workdir_prefixes to assert calls and
responses. Ensure tests reference add_allowed_dir,
_runtime_allowlist_mutation_enabled, _DENIED_PATHS, _save_runtime_allowed_dir,
and _allowed_workdir_prefixes.
In `@web/src/app/studio/page.tsx`:
- Around line 108-111: The generate flow fails when users submit inline
title+abstract because generate() still requires paper_id; update the logic so
after creating or selecting a paper via addPaper({ title, abstract }) you pass
the resulting/selected paper's id into generate() when paper_id is missing.
Concretely, ensure the branch that calls addPaper stores or awaits the created
paper id (from addPaper or its callback) and then calls generate(paper_id) (or
uses that id where generate() is invoked), and modify the block around the
existing paper_id checks (the code that currently hard-fails when paper_id is
absent in the generate path) to use the newly created paper id instead of
aborting.
- Around line 85-159: The new useEffect in page.tsx introduces
query-param-driven behavior (reading paper_id, title, abstract, generate,
context_pack_id), context pack loading (setContextPack, setContextPackError,
setContextPackLoading), paper creation/selection (addPaper, selectPaper) and
generate triggering (generate) plus URL cleaning via router.replace, but no
tests were added; add/update unit/integration tests to cover: 1) ingestion of
query params to create or select papers (paper_id, title, abstract) and that
hasProcessedParams prevents reprocessing, 2) context pack fetch flow including
success (normalizes via normalizePack and calls setContextPack) and failure
(sets contextPackError and loading toggles), 3) generate path when generate=true
both with and without paper_id to ensure generate({ paperId }) is called and
missing id sets contextPackError, and 4) rendering differences between gallery
and workspace based on selectedPaperId; target tests at the component level for
the Studio page (page.tsx) mocking fetch/responses and router.searchParams, and
assert router.replace("/studio") is called when shouldCleanUrl is true.
In `@web/src/components/studio/NewPaperModal.tsx`:
- Around line 95-152: Add unit/integration tests covering duplicate detection
and messaging for both manual and library imports: for handleManualSubmit, test
that submitting a title that matches an existing paper
(case/whitespace-insensitive) sets the error "A paper with this title already
exists in the studio." and does not call addPaper or resetAndClose; also test
successful submission calls addPaper with trimmed fields and calls
resetAndClose. For handleImportSelected, test multi-select behavior where some
selected library papers are already in the studio by id or title
(case/whitespace-insensitive): ensure duplicates are skipped, non-duplicates are
added (addPaper called for each), existingTitles is updated, and when all
selected are duplicates the correct singular vs plural error messages are set
("This paper already exists..." vs "The selected papers already exist...") and
resetAndClose is not called. Reference handleManualSubmit, handleImportSelected,
addPaper, resetAndClose, selectedPaperIds, libraryPapers, and papers when
writing tests.
In `@web/src/components/studio/ReproductionLog.tsx`:
- Around line 315-385: Add integration tests for the new CLI-event → action
mapping in ReproductionLog.tsx: write tests that feed mocked SSE events
(simulate readSSE stream) with cli_event values "text", "tool_use",
"tool_result", "thinking", a keepalive payload, legacy message, and fallback
"delta", then assert that addAction/appendToLastAction are called with the
correct action types/contents and metadata, that updateTaskStatus/setStatus are
invoked for "result" and "error" events, and that keepalive is ignored; use the
component/store test harness to mount the component (or call the handler) and
verify action order, merging of streaming text into the last text action,
attaching tool_result to the preceding function_call, and proper error handling
via setLastError.
In `@web/src/hooks/useContextPackGeneration.ts`:
- Around line 64-97: Add unit tests for the useContextPackGeneration hook
focusing on the generate function: one test should assert the POST payload sent
to "/api/research/repro/context" contains paper_id, user_id (defaulting to
"default" when absent), depth (defaulting to "standard"), and optional
title/abstract only when provided; another test should exercise error handling
to ensure error-message precedence (e.g., prefer a parsed error message from the
response body over a generic fetch/network error) so regressions in
request-shaping and error extraction are caught.
In `@web/src/lib/store/project-context.ts`:
- Around line 35-40: The updateFile updater may produce a malformed VirtualFile
when state.files[name] is undefined; modify updateFile to first check whether
state.files[name] exists and either (a) merge content into the existing file ({
...state.files[name], content }) or (b) create a proper VirtualFile with
required properties (include name and a sensible language fallback, e.g.,
state.files[name]?.language || 'text') before setting it; reference the
updateFile function and the VirtualFile shape when implementing this guard so
callers never get an entry missing name or language.
In `@web/src/lib/store/studio-store.ts`:
- Around line 188-218: addPaper currently switches selectedPaperId and clears
per-paper scoped state without persisting the current selected paper's transient
data, causing loss of contextPack, generationProgress, liveObservations,
lastGenCodeResult, workspaceSnapshotId, and related flags; update addPaper to
persist the current paper's per-paper state before changing selection by reusing
the same save logic used in selectPaper (or call a extracted helper the two
share) so that the outgoing paper's contextPack, contextPackLoading,
contextPackError, generationProgress, liveObservations, lastGenCodeResult, and
workspaceSnapshotId are stored (e.g., via the savePapersToStorage/ per-paper
save flow) before setting selectedPaperId and resetting paperDraft and scoped
fields for the new paper.
---
Nitpick comments:
In `@web/src/components/studio/PaperIcon.tsx`:
- Around line 72-74: The SVG uses an ambiguous two-letter `abbr` for aria-label;
in the PaperIcon component update the SVG `aria-label` to the full paper title
(e.g., use the `title`/`paperTitle` prop if available) and fall back to `abbr`
only if the full title is missing, and apply the same change to the other SVG
instance referenced around the 87-99 area; modify the PaperIcon props or usage
so `title`/`paperTitle` is passed through and used for accessibility while
keeping `abbr` as a fallback.
In `@web/src/components/studio/ReproductionLog.tsx`:
- Around line 341-351: The "tool_result" branch is incorrectly adding a new
function_call action instead of updating the most recent tool invocation; change
the logic so that when cliEvent === "tool_result" you locate the most recent
action for the given taskId with type "function_call" (use whatever action
list/storage is used by addAction/getActions) and update that action's
metadata.result and content rather than calling addAction again; adjust usage of
lastActionIsText as needed to reflect that the existing action was updated; use
identifiers like addAction, lastActionIsText, taskId, and the "function_call"
action type to find and modify the correct entry.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
src/paperbot/api/routes/repro_context.pysrc/paperbot/api/routes/runbook.pysrc/paperbot/api/routes/studio_chat.pyweb/next.config.tsweb/src/app/studio/page.tsxweb/src/components/studio/NewPaperModal.tsxweb/src/components/studio/PaperGallery.tsxweb/src/components/studio/PaperIcon.tsxweb/src/components/studio/ReproductionLog.tsxweb/src/hooks/useContextPackGeneration.tsweb/src/lib/store/project-context.tsweb/src/lib/store/studio-store.ts
Comment on lines
+120
to
+131
| try: | ||
| await asyncio.to_thread( | ||
| _store.save_stage_result, | ||
| pack_id=pack_id, | ||
| stage_name=stage_name, | ||
| status="completed", | ||
| result_data={"observations": [o.to_full() for o in observations]}, | ||
| confidence=confidence, | ||
| duration_ms=0, | ||
| ) | ||
| except Exception: | ||
| pass |
There was a problem hiding this comment.
Avoid silent persistence failures in SSE path.
Current except Exception: pass blocks hide DB failures entirely. The stream should remain non-fatal, but persistence failures should still be logged for diagnosis.
🛠️ Suggested fix
- except Exception:
- pass
+ except Exception as exc:
+ Logger.warning(
+ f"[M2] stage_persist_failed pack_id={pack_id} stage={stage_name} error={exc}",
+ file=LogFiles.ERROR,
+ )
@@
- except Exception:
- pass
+ except Exception as exc:
+ Logger.warning(
+ f"[M2] store_mark_failed_error pack_id={pack_id} error={exc}",
+ file=LogFiles.ERROR,
+ )Also applies to: 207-210
🧰 Tools
🪛 Ruff (0.15.2)
[error] 130-131: try-except-pass detected, consider logging the exception
(S110)
[warning] 130-130: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/paperbot/api/routes/repro_context.py` around lines 120 - 131, The
try/except around the asyncio.to_thread call to _store.save_stage_result
currently swallows all exceptions (except Exception: pass); change it to catch
Exception as e and log the failure (including pack_id and stage_name) using the
module logger (e.g., logger.exception or logger.error with exception info) so
persistence failures in the SSE path are recorded but do not break the stream;
apply the same change to the identical block that appears around the other
save_stage_result call (the one referenced at lines 207-210).
Comment on lines
+169
to
+176
| if request.title and request.abstract: | ||
| inline_raw = RawPaperData( | ||
| paper_id=request.paper_id, | ||
| title=request.title, | ||
| abstract=request.abstract, | ||
| source_adapter="inline", | ||
| ) | ||
|
|
There was a problem hiding this comment.
Inline mode is skipped when abstract is intentionally empty.
The truthy check requires both fields to be non-empty strings. For studio papers with empty abstracts, this falls back to input-router lookup (which cannot resolve studio IDs), defeating the inline bypass.
✅ Suggested fix
- if request.title and request.abstract:
+ if request.title is not None and request.abstract is not None and request.title.strip():
inline_raw = RawPaperData(
paper_id=request.paper_id,
- title=request.title,
- abstract=request.abstract,
+ title=request.title.strip(),
+ abstract=request.abstract,
source_adapter="inline",
)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/paperbot/api/routes/repro_context.py` around lines 169 - 176, Summary:
Inline mode is skipped when abstract == "" because the condition requires both
title and abstract to be truthy; change the check to allow empty abstracts.
Update the conditional that constructs RawPaperData (look for request.title,
request.abstract, inline_raw) so it requires a non-empty title but treats
abstract as present when it is not None (e.g., use "request.title and
request.abstract is not None" or equivalent), then proceed to build RawPaperData
with paper_id, title, abstract, source_adapter="inline" so studio papers with
empty abstracts still use inline bypass.
|
|
||
| def _runtime_allowlist_mutation_enabled() -> bool: | ||
| return os.getenv("PAPERBOT_RUNBOOK_ALLOWLIST_MUTATION", "false").lower() == "true" | ||
| return os.getenv("PAPERBOT_RUNBOOK_ALLOWLIST_MUTATION", "true").lower() == "true" |
There was a problem hiding this comment.
Keep runtime allowlist mutation opt-in by default.
Defaulting this flag to enabled weakens secure defaults for a filesystem-privileged endpoint.
Suggested patch
def _runtime_allowlist_mutation_enabled() -> bool:
- return os.getenv("PAPERBOT_RUNBOOK_ALLOWLIST_MUTATION", "true").lower() == "true"
+ return os.getenv("PAPERBOT_RUNBOOK_ALLOWLIST_MUTATION", "false").strip().lower() in {
+ "1",
+ "true",
+ "yes",
+ "on",
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/paperbot/api/routes/runbook.py` at line 215, The runtime
allowlist-mutation flag currently defaults to enabled by returning
os.getenv("PAPERBOT_RUNBOOK_ALLOWLIST_MUTATION", "true").lower() == "true";
change the default to disabled by using "false" as the fallback so the
environment opt-in is required (i.e., set default to "false" for the
PAPERBOT_RUNBOOK_ALLOWLIST_MUTATION getenv call), and update any related tests
or documentation that assume the default was true to reflect the new secure
default.
Comment on lines
246
to
257
| if str(resolved) in _DENIED_PATHS: | ||
| raise HTTPException( | ||
| status_code=403, | ||
| detail=f"adding '{resolved}' is not allowed — path is too broad or sensitive" | ||
| detail=f"adding '{resolved}' is not allowed — path is too broad or sensitive", | ||
| ) | ||
|
|
||
| # Deny home directory itself — too broad | ||
| if resolved == Path.home().resolve(): | ||
| raise HTTPException( | ||
| status_code=403, | ||
| detail="adding home directory is not allowed — too broad", | ||
| ) |
There was a problem hiding this comment.
Denylist enforcement is too narrow (exact-match only).
Current logic blocks only exact paths in _DENIED_PATHS; descendants like /etc/ssh or /proc/sys can still be allowlisted.
Suggested patch
- if str(resolved) in _DENIED_PATHS:
+ if any(_is_under_prefix(resolved, Path(denied)) for denied in _DENIED_PATHS):
raise HTTPException(
status_code=403,
detail=f"adding '{resolved}' is not allowed — path is too broad or sensitive",
)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/paperbot/api/routes/runbook.py` around lines 246 - 257, The current
exact-match check against _DENIED_PATHS and the home-directory check allow
descendants (e.g., /etc/ssh) through; change them to reject any path that is the
same as or a descendant of a denied path or of the home dir by resolving each
denied entry and using Path.is_relative_to (or equivalent) against resolved
(i.e., replace "if str(resolved) in _DENIED_PATHS" with a loop/any that compares
resolved == denied_resolved or resolved.is_relative_to(denied_resolved) for each
denied entry in _DENIED_PATHS, and update the home check to reject when resolved
== home_resolved or resolved.is_relative_to(home_resolved)).
Comment on lines
+250
to
+327
| def _parse_cli_content_blocks(content_blocks: list) -> list[StreamEvent]: | ||
| """Convert Claude CLI assistant content blocks into StreamEvents.""" | ||
| events: list[StreamEvent] = [] | ||
| for block in content_blocks: | ||
| btype = block.get("type", "") | ||
|
|
||
| if btype == "text": | ||
| text = block.get("text", "") | ||
| if text: | ||
| events.append(StreamEvent( | ||
| type="progress", | ||
| data={"cli_event": "text", "text": text}, | ||
| )) | ||
|
|
||
| elif btype == "tool_use": | ||
| events.append(StreamEvent( | ||
| type="progress", | ||
| data={ | ||
| "cli_event": "tool_use", | ||
| "tool_name": block.get("name", "unknown"), | ||
| "tool_input": block.get("input", {}), | ||
| "tool_id": block.get("id", ""), | ||
| }, | ||
| )) | ||
|
|
||
| elif btype == "thinking": | ||
| thinking = block.get("thinking", "") | ||
| if thinking: | ||
| events.append(StreamEvent( | ||
| type="progress", | ||
| data={"cli_event": "thinking", "text": thinking}, | ||
| )) | ||
|
|
||
| return events | ||
|
|
||
|
|
||
| def _parse_cli_event(line_data: Dict[str, Any]) -> list[StreamEvent]: | ||
| """Parse a single NDJSON line from `claude -p --output-format stream-json`. | ||
|
|
||
| Claude CLI stream-json emits one JSON object per line: | ||
| - {"type":"assistant","message":{...}} — assistant turn with content blocks | ||
| - {"type":"tool_result","tool_name":"...","content":"..."} — tool output | ||
| - {"type":"result","subtype":"success","result":"...","cost_usd":...} — final | ||
| - {"type":"system",...} — session init (ignored) | ||
| """ | ||
| etype = line_data.get("type", "") | ||
| events: list[StreamEvent] = [] | ||
|
|
||
| if etype == "assistant": | ||
| msg = line_data.get("message", {}) | ||
| content_blocks = msg.get("content", []) | ||
| events.extend(_parse_cli_content_blocks(content_blocks)) | ||
|
|
||
| elif etype == "tool_result": | ||
| events.append(StreamEvent( | ||
| type="progress", | ||
| data={ | ||
| "cli_event": "tool_result", | ||
| "tool_name": line_data.get("tool_name", ""), | ||
| "content": _truncate(str(line_data.get("content", "")), 2000), | ||
| }, | ||
| )) | ||
|
|
||
| elif etype == "result": | ||
| events.append(StreamEvent( | ||
| type="result", | ||
| data={ | ||
| "cli_event": "done", | ||
| "result": line_data.get("result", ""), | ||
| "cost_usd": line_data.get("cost_usd"), | ||
| "duration_ms": line_data.get("duration_ms"), | ||
| "num_turns": line_data.get("num_turns"), | ||
| }, | ||
| )) | ||
|
|
||
| # Ignore "system" and other meta events | ||
| return events | ||
|
|
There was a problem hiding this comment.
Add tests for NDJSON event parsing and keepalive behavior.
This introduces a new streaming contract (cli_event variants, line-buffer parsing, trailing-buffer parse, keepalive emissions). It should be locked down with route-level tests.
As per coding guidelines {src,web/src}/**/*.{py,ts,tsx}: If behavior changes, add or update tests.
Also applies to: 395-437
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/paperbot/api/routes/studio_chat.py` around lines 250 - 327, The new
streaming contract for Claude CLI output (cli_event variants, line-buffer
parsing, trailing-buffer parse, and keepalive emissions) isn't covered by tests;
add route-level tests that exercise _parse_cli_content_blocks and
_parse_cli_event behavior (including "assistant" content blocks, "tool_result",
"result", ignored "system"), validate StreamEvent objects produced (cli_event
values like "text","thinking","tool_use","tool_result","done"), ensure _truncate
is applied to long tool content, and add tests that simulate line-buffered and
trailing-buffer NDJSON inputs plus keepalive/heartbeat emissions to assert
correct event emission order and resilience to partial lines.
Comment on lines
+118
to
+130
| const existingIds = new Set(papers.map(p => p.id)) | ||
| const existingTitles = new Set(papers.map(p => p.title.trim().toLowerCase())) | ||
|
|
||
| for (const paperId of selectedPaperIds) { | ||
| // Skip if already in studio | ||
| if (existingIds.has(paperId)) continue | ||
| let importedCount = 0 | ||
| const skippedTitles: string[] = [] | ||
|
|
||
| for (const paperId of selectedPaperIds) { | ||
| const paper = libraryPapers.find(p => p.id === paperId) | ||
| if (paper) { | ||
| addPaper({ | ||
| title: paper.title, | ||
| abstract: paper.abstract || "", | ||
| }) | ||
| if (!paper) continue | ||
|
|
||
| // Skip if already in studio (by ID or title) | ||
| if (existingIds.has(paperId) || existingTitles.has(paper.title.trim().toLowerCase())) { | ||
| skippedTitles.push(paper.title) |
There was a problem hiding this comment.
ID-based duplicate detection does not actually work with current StudioPaper.id semantics.
existingIds is built from studio-local IDs, while paperId is a library ID. This means the ID branch of dedupe never reliably matches and only title dedupe is effectively enforced.
💡 Minimal fix (until source IDs are persisted)
- const existingIds = new Set(papers.map(p => p.id))
const existingTitles = new Set(papers.map(p => p.title.trim().toLowerCase()))
@@
- if (existingIds.has(paperId) || existingTitles.has(paper.title.trim().toLowerCase())) {
+ if (existingTitles.has(paper.title.trim().toLowerCase())) {
skippedTitles.push(paper.title)
continue
}If ID-level dedupe is required, persist a dedicated source/library ID on StudioPaper and compare against that field.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const existingIds = new Set(papers.map(p => p.id)) | |
| const existingTitles = new Set(papers.map(p => p.title.trim().toLowerCase())) | |
| for (const paperId of selectedPaperIds) { | |
| // Skip if already in studio | |
| if (existingIds.has(paperId)) continue | |
| let importedCount = 0 | |
| const skippedTitles: string[] = [] | |
| for (const paperId of selectedPaperIds) { | |
| const paper = libraryPapers.find(p => p.id === paperId) | |
| if (paper) { | |
| addPaper({ | |
| title: paper.title, | |
| abstract: paper.abstract || "", | |
| }) | |
| if (!paper) continue | |
| // Skip if already in studio (by ID or title) | |
| if (existingIds.has(paperId) || existingTitles.has(paper.title.trim().toLowerCase())) { | |
| skippedTitles.push(paper.title) | |
| const existingTitles = new Set(papers.map(p => p.title.trim().toLowerCase())) | |
| let importedCount = 0 | |
| const skippedTitles: string[] = [] | |
| for (const paperId of selectedPaperIds) { | |
| const paper = libraryPapers.find(p => p.id === paperId) | |
| if (!paper) continue | |
| // Skip if already in studio (by ID or title) | |
| if (existingTitles.has(paper.title.trim().toLowerCase())) { | |
| skippedTitles.push(paper.title) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/components/studio/NewPaperModal.tsx` around lines 118 - 130,
existingIds is built from StudioPaper.id but selectedPaperIds are library IDs,
so the ID-based dedupe in the NewPaperModal import loop never matches; update
the StudioPaper model to persist the source/library ID (e.g., add a sourceId or
libraryId field) and change the dedupe check to compare selectedPaperIds against
that persisted field (replace existingIds construction to use studioPapers.map(p
=> p.sourceId) and ensure code that creates StudioPaper sets sourceId from the
library paper), or if you cannot persist yet, remove the ID branch and rely on
title dedupe only until you add the sourceId field.
Comment on lines
+95
to
+104
| await fetch("/api/runbook/delete", { | ||
| method: "POST", | ||
| headers: { "Content-Type": "application/json" }, | ||
| body: JSON.stringify({ | ||
| project_dir: paperToDelete.outputDir, | ||
| }), | ||
| }) | ||
| } catch (e) { | ||
| console.error("Failed to delete project files:", e) | ||
| } |
There was a problem hiding this comment.
HTTP failures from file-deletion API are currently swallowed.
fetch won’t throw on 4xx/5xx, so this path can silently proceed as if cleanup succeeded. At minimum, check res.ok and log/report when deletion fails.
✅ Suggested fix
- await fetch("/api/runbook/delete", {
+ const res = await fetch("/api/runbook/delete", {
method: "POST",
headers: { "Content-Type": "application/json" },
body: JSON.stringify({
project_dir: paperToDelete.outputDir,
}),
})
+ if (!res.ok) {
+ const text = await res.text().catch(() => "")
+ console.warn("Failed to delete project files:", res.status, text)
+ }
} catch (e) {
console.error("Failed to delete project files:", e)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| await fetch("/api/runbook/delete", { | |
| method: "POST", | |
| headers: { "Content-Type": "application/json" }, | |
| body: JSON.stringify({ | |
| project_dir: paperToDelete.outputDir, | |
| }), | |
| }) | |
| } catch (e) { | |
| console.error("Failed to delete project files:", e) | |
| } | |
| const res = await fetch("/api/runbook/delete", { | |
| method: "POST", | |
| headers: { "Content-Type": "application/json" }, | |
| body: JSON.stringify({ | |
| project_dir: paperToDelete.outputDir, | |
| }), | |
| }) | |
| if (!res.ok) { | |
| const text = await res.text().catch(() => "") | |
| console.warn("Failed to delete project files:", res.status, text) | |
| } | |
| } catch (e) { | |
| console.error("Failed to delete project files:", e) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/components/studio/PaperGallery.tsx` around lines 95 - 104, The fetch
call triggered when deleting a paper in PaperGallery.tsx currently ignores HTTP
error responses; update the delete flow that uses paperToDelete and the
"/api/runbook/delete" POST so it checks the fetch response (res.ok) and handles
non-ok responses by logging or throwing a descriptive error (including status
and body text) inside the try/catch, and propagate or surface the failure to the
UI instead of treating all responses as success.
Comment on lines
+87
to
+88
| if (params.title) payload.title = params.title | ||
| if (params.abstract) payload.abstract = params.abstract |
There was a problem hiding this comment.
Truthy checks can drop intentionally empty inline fields.
Using truthiness here omits abstract: "", which can disable server-side inline handling for papers whose abstract is empty but still valid input.
✅ Suggested fix
- if (params.title) payload.title = params.title
- if (params.abstract) payload.abstract = params.abstract
+ if (params.title !== undefined) payload.title = params.title
+ if (params.abstract !== undefined) payload.abstract = params.abstract📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (params.title) payload.title = params.title | |
| if (params.abstract) payload.abstract = params.abstract | |
| if (params.title !== undefined) payload.title = params.title | |
| if (params.abstract !== undefined) payload.abstract = params.abstract |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/hooks/useContextPackGeneration.ts` around lines 87 - 88, The truthy
checks in useContextPackGeneration (the payload building that sets payload.title
and payload.abstract from params.title and params.abstract) drop intentionally
empty strings like ""; change the checks to test for undefined (e.g.,
params.abstract !== undefined) or check hasOwnProperty before assigning so empty
but valid values are preserved and still sent to the server; update both the
title and abstract assignments in the payload construction to use these
explicit-presence checks.
Comment on lines
+104
to
+112
| // Per-paper cached state (preserved across paper switches) | ||
| interface PerPaperCache { | ||
| contextPack: ReproContextPack | null | ||
| contextPackLoading: boolean | ||
| contextPackError: string | null | ||
| generationProgress: StageProgressEvent[] | ||
| liveObservations: StageObservationsEvent[] | ||
| activeTaskId: string | null | ||
| } |
There was a problem hiding this comment.
Please add store tests for per-paper cache round-trip and streaming append behavior.
The new caching contract and appendToLastAction are core interaction paths and should be covered directly.
As per coding guidelines {src,web/src}/**/*.{py,ts,tsx}: If behavior changes, add or update tests.
Also applies to: 262-299, 371-381
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/lib/store/studio-store.ts` around lines 104 - 112, Add unit tests
that verify the PerPaperCache round-trip and the streaming-append behavior of
appendToLastAction: 1) create a PerPaperCache instance (exercise keys
contextPack, contextPackLoading, contextPackError, generationProgress,
liveObservations, activeTaskId), save it into the store, switch papers (simulate
paper-id change) and assert the cache for the original paper is preserved and
can be restored exactly; 2) test appendToLastAction by dispatching/committing an
action that creates an initial action item, then call appendToLastAction
multiple times with incremental chunks and assert the last action’s content and
metadata are updated incrementally (including edge cases: no existing action,
empty chunk), and that generationProgress/liveObservations updates behave like
streaming appends; use the existing studio-store methods and state selectors to
locate PerPaperCache and appendToLastAction in the store for assertions.
Comment on lines
249
to
257
| ...(state.selectedPaperId === paperId ? { | ||
| paperDraft: { title: '', abstract: '', methodSection: '' }, | ||
| lastGenCodeResult: null, | ||
| contextPack: null, | ||
| contextPackLoading: false, | ||
| contextPackError: null, | ||
| generationProgress: [], | ||
| liveObservations: [], | ||
| } : {}), |
There was a problem hiding this comment.
activeTaskId should be cleared when deleting the selected paper.
Current cleanup resets per-paper context fields but keeps the previously active task selected, which can leave stale task context visible.
✅ Suggested fix
...(state.selectedPaperId === paperId ? {
paperDraft: { title: '', abstract: '', methodSection: '' },
lastGenCodeResult: null,
+ activeTaskId: null,
contextPack: null,
contextPackLoading: false,
contextPackError: null,
generationProgress: [],
liveObservations: [],📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ...(state.selectedPaperId === paperId ? { | |
| paperDraft: { title: '', abstract: '', methodSection: '' }, | |
| lastGenCodeResult: null, | |
| contextPack: null, | |
| contextPackLoading: false, | |
| contextPackError: null, | |
| generationProgress: [], | |
| liveObservations: [], | |
| } : {}), | |
| ...(state.selectedPaperId === paperId ? { | |
| paperDraft: { title: '', abstract: '', methodSection: '' }, | |
| lastGenCodeResult: null, | |
| activeTaskId: null, | |
| contextPack: null, | |
| contextPackLoading: false, | |
| contextPackError: null, | |
| generationProgress: [], | |
| liveObservations: [], | |
| } : {}), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/lib/store/studio-store.ts` around lines 249 - 257, When removing a
paper (the conditional block that checks state.selectedPaperId === paperId and
resets paperDraft, lastGenCodeResult, contextPack, etc.), also clear the
per-paper active task by setting activeTaskId to null so stale task context
isn't retained; update the cleanup object inside that conditional (the same
place where paperDraft, lastGenCodeResult, contextPackLoading, contextPackError,
generationProgress, and liveObservations are cleared) to include activeTaskId:
null.
…I chat errors - 修改Gemini AI review代码问题
…I chat errors - 修改Gemini AI review代码问题
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
jerry609
added a commit
that referenced
this pull request
Mar 4, 2026
* Feat/daily push epic 179 (#218) * feat(push): implement Epic #179 — daily push optimization Multi-channel push infrastructure with structured digest cards: - #187: Integrate HF upvotes into Judge scoring prompt - #180: Add digest_card extraction (highlight/method/finding/tags) with new LLM prompt, email template rendering, markdown output - #181: MinerU Cloud API client for PDF figure extraction - #182: Apprise multi-channel push layer with YAML config - #183-185: Channel formatters (Telegram MarkdownV2, Discord Rich Embed, WeCom markdown+news, Feishu/Lark interactive card) - #186: RSS 2.0 + Atom feed endpoints (/api/feed/daily.xml, .atom) Also creates follow-up issues #190 (Push Channel Settings UI) and #191 (Dashboard Digest Card Enhancement). 56 new tests, all passing. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat: integrate MinerU figure extraction into CLI and ARQ worker daily-paper workflow Refs #179 * fix(daily-push): enable MinerU flags in ARQ cron payload (refs #181) * feat(push): wire channel formatters into apprise digest delivery (refs #182 #183 #184 #185) * feat(feed): add keyword RSS, feed cache, and figure enclosure support (refs #186) * feat(mineru): add extraction result cache for daily push figure pipeline (refs #181) * feat(push): add telegram bot command subscription endpoint (refs #191) * fix(push): add retry/backoff for channel delivery failures (refs #191) * feat(hf): add trending modes, cache, and arxiv identity dedupe (refs #193) * feat(push): add idempotency and error-code mapping for channel delivery (refs #191) * test(push): add mock channel e2e and failure-injection coverage (refs #191) * docs(push): add audit script and ops runbook for acceptance closure (refs #192) * docs: update README for AgentSwarm execution transition * docs: archive stale docs and rename AgentSwarm todo * docs: add AgentSwarm design proposal + web Playwright e2e setup - Add docs/proposals/agentswarm-design.md covering multi-agent orchestration architecture, adapter pattern, session management, skills ecosystem, and OpenCode/OpenClaw/oMo integration (refs #197, #214) - Add Playwright e2e test scaffolding for web dashboard (smoke, papers, research specs) - Update web/.gitignore for Playwright artifacts Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> * fix(studio): fix context pack generation, duplicate import UX, and CLI chat errors (#223) * fix(studio): fix context pack generation, duplicate import UX, and CLI chat errors - 支持内联标题/摘要，以绕过 PaperInputRouter 获取 studio 论文 ID - 使上下文包生成 SSE 流期间的数据库持久化操作不再致命 - 从库中导入重复论文时显示错误消息 - 将左侧文件面板宽度缩小至约 18%（默认宽度）；移除默认模板文件 - 为 Claude CLI 添加 --verbose 标志，以兼容 stream-json 输出 Related to #222 * fix(studio): fix context pack generation, duplicate import UX, and CLI chat errors - 修改Gemini AI review代码问题 * fix(studio): fix context pack generation, duplicate import UX, and CLI chat errors - 修改Gemini AI review代码问题 --------- Co-authored-by: Jerry Zhang <1772030600@qq.com> * fix(web): correct package.json scripts JSON syntax * chore(gitignore): block nested .env secrets and keep examples * docs(env): add MinerU key and figure extraction env templates * refactor(mineru): standardize on v4 task API for daily push (#229) * fix(mineru): support v4 async task extraction flow * refactor(mineru): standardize on v4 task API for daily push (#179) * feat(push): render MinerU main figure in email via inline fallback * docs(readme): update email push demo screenshot * docs(readme): refresh Phase 6 daily push descriptions * fix(security): harden runbook allowed-dir path validation * fix(review): resolve security and reliability findings from AI review * fix(codeql): harden studio chat project_dir normalization --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Boyu liu <oor2020@163.com>
jerry609
added a commit
that referenced
this pull request
Mar 4, 2026
* Feat/daily push epic 179 (#218) * feat(push): implement Epic #179 — daily push optimization Multi-channel push infrastructure with structured digest cards: - #187: Integrate HF upvotes into Judge scoring prompt - #180: Add digest_card extraction (highlight/method/finding/tags) with new LLM prompt, email template rendering, markdown output - #181: MinerU Cloud API client for PDF figure extraction - #182: Apprise multi-channel push layer with YAML config - #183-185: Channel formatters (Telegram MarkdownV2, Discord Rich Embed, WeCom markdown+news, Feishu/Lark interactive card) - #186: RSS 2.0 + Atom feed endpoints (/api/feed/daily.xml, .atom) Also creates follow-up issues #190 (Push Channel Settings UI) and #191 (Dashboard Digest Card Enhancement). 56 new tests, all passing. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat: integrate MinerU figure extraction into CLI and ARQ worker daily-paper workflow Refs #179 * fix(daily-push): enable MinerU flags in ARQ cron payload (refs #181) * feat(push): wire channel formatters into apprise digest delivery (refs #182 #183 #184 #185) * feat(feed): add keyword RSS, feed cache, and figure enclosure support (refs #186) * feat(mineru): add extraction result cache for daily push figure pipeline (refs #181) * feat(push): add telegram bot command subscription endpoint (refs #191) * fix(push): add retry/backoff for channel delivery failures (refs #191) * feat(hf): add trending modes, cache, and arxiv identity dedupe (refs #193) * feat(push): add idempotency and error-code mapping for channel delivery (refs #191) * test(push): add mock channel e2e and failure-injection coverage (refs #191) * docs(push): add audit script and ops runbook for acceptance closure (refs #192) * docs: update README for AgentSwarm execution transition * docs: archive stale docs and rename AgentSwarm todo * docs: add AgentSwarm design proposal + web Playwright e2e setup - Add docs/proposals/agentswarm-design.md covering multi-agent orchestration architecture, adapter pattern, session management, skills ecosystem, and OpenCode/OpenClaw/oMo integration (refs #197, #214) - Add Playwright e2e test scaffolding for web dashboard (smoke, papers, research specs) - Update web/.gitignore for Playwright artifacts Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> * fix(studio): fix context pack generation, duplicate import UX, and CLI chat errors (#223) * fix(studio): fix context pack generation, duplicate import UX, and CLI chat errors - 支持内联标题/摘要，以绕过 PaperInputRouter 获取 studio 论文 ID - 使上下文包生成 SSE 流期间的数据库持久化操作不再致命 - 从库中导入重复论文时显示错误消息 - 将左侧文件面板宽度缩小至约 18%（默认宽度）；移除默认模板文件 - 为 Claude CLI 添加 --verbose 标志，以兼容 stream-json 输出 Related to #222 * fix(studio): fix context pack generation, duplicate import UX, and CLI chat errors - 修改Gemini AI review代码问题 * fix(studio): fix context pack generation, duplicate import UX, and CLI chat errors - 修改Gemini AI review代码问题 --------- Co-authored-by: Jerry Zhang <1772030600@qq.com> * fix(web): correct package.json scripts JSON syntax * chore(gitignore): block nested .env secrets and keep examples * docs(env): add MinerU key and figure extraction env templates * refactor(mineru): standardize on v4 task API for daily push (#229) * fix(mineru): support v4 async task extraction flow * refactor(mineru): standardize on v4 task API for daily push (#179) * feat(push): render MinerU main figure in email via inline fallback * docs(readme): update email push demo screenshot * docs(readme): refresh Phase 6 daily push descriptions * fix(security): harden runbook allowed-dir path validation * fix(review): resolve security and reliability findings from AI review * fix(codeql): harden studio chat project_dir normalization * docs(readme): restructure README for professional presentation - Add centered hero header with badges (CI, Roadmap, License, Python, Next.js) - Replace verbose 18-row feature table with categorized bullet lists - Move Roadmap Phase 1-6 content to pinned issue #232 - Move module maturity matrix to Roadmap #232 - Consolidate screenshots into collapsible sections - Remove inline API endpoint table (40+ rows) and directory tree - Add Contributing section with Roadmap link - Streamline Getting Started with collapsible config details - Add repo topics for GitHub discoverability References: vLLM, Docling, PaperQA2, R2R README patterns Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Boyu liu <oor2020@163.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
支持内联标题/摘要，以绕过 PaperInputRouter 获取 studio 论文 ID
使上下文包生成 SSE 流期间的数据库持久化操作不再致命
从库中导入重复论文时显示错误消息
将左侧文件面板宽度缩小至约 18%（默认宽度）；移除默认模板文件
为 Claude CLI 添加 --verbose 标志，以兼容 stream-json 输出
Related to #222
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Improvements