Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (36)
📝 WalkthroughWalkthroughThe PR fundamentally restructures PaperBot from a Claude Code-specific orchestration system to an agent-agnostic dashboard. It introduces a unified EventType vocabulary and event envelope system, implements real-time SSE-based event streaming with a bounded Zustand store, develops a comprehensive three-panel UI dashboard with activity feed and file diffs, adds a Kanban board for task visualization with Codex delegation tracking, and consolidates Next.js API routes via a centralized backend proxy utility. Extensive planning documentation details v1.2 milestones and the shift toward agent-agnostic architecture. Changes
Sequence DiagramsequenceDiagram
participant Agent as Agent/MCP
participant EventBus as EventBus<br/>(Python)
participant SSE as SSE Endpoint<br/>(/api/events/stream)
participant Hook as useAgentEvents<br/>(React Hook)
participant Store as Zustand Store<br/>(agent-events)
participant UI as UI Components<br/>(Activity, Dashboard)
Agent->>EventBus: emit_lifecycle_event<br/>(AGENT_STARTED)
Agent->>EventBus: emit_tool_call_event<br/>(TOOL_RESULT)
EventBus->>SSE: append event to stream
Hook->>SSE: readSSE() with AbortController
SSE-->>Hook: text/event-stream message
Hook->>Hook: parseActivityItem(raw)
Hook->>Hook: parseToolCall(raw)
Hook->>Hook: parseFileTouched(raw)
Hook->>Hook: parseCodexDelegation(raw)
Hook->>Store: addFeedItem(parsed)
Hook->>Store: addToolCall(parsed)
Hook->>Store: addFileTouched(parsed)
Hook->>Store: addCodexDelegation(parsed)
Store-->>UI: feed (capped 200)
Store-->>UI: toolCalls (capped 100)
Store-->>UI: filesTouched (20 runs)
Store-->>UI: codexDelegations (capped 100)
UI->>UI: ActivityFeed renders feed items
UI->>UI: ToolCallTimeline displays calls
UI->>UI: FileListPanel lists changes
UI->>UI: KanbanBoard shows task status
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
Vercel Preview
|
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 refactors the Next.js API layer by introducing a unified backend proxy, streamlining frontend-to-backend communication and reducing code duplication. It also lays substantial groundwork for an agent-agnostic dashboard, enhancing real-time observability of agent activities, tool calls, and file changes through new frontend components and an extended event vocabulary. The changes include new UI elements for task management and detailed codebase documentation, preparing the project for advanced multi-agent orchestration features. 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
|
![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
This pull request is a significant and well-executed refactoring that unifies the Next.js API backend proxies into a shared helper, greatly reducing code duplication and improving maintainability. The introduction of contract tests to enforce this new pattern is an excellent addition. The changes are clean and the new proxy abstraction is well-designed. I have one suggestion to improve debuggability in the new event emission helper.
Comment on lines
+120
to
+121
| except Exception as exc: # pragma: no cover | ||
| log.debug("_emit_codex_event failed silently: %s", exc) |
There was a problem hiding this comment.
To improve debuggability, it's a good practice to include the exception traceback when logging caught exceptions. Using exc_info=True will automatically add the traceback to the log record, which can be invaluable for diagnosing why event emission might be failing silently.
Suggested change
| except Exception as exc: # pragma: no cover | |
| log.debug("_emit_codex_event failed silently: %s", exc) | |
| log.debug("_emit_codex_event failed silently: %s", exc, exc_info=True) |
There was a problem hiding this comment.
Pull request overview
This PR centralizes Next.js API route “backend proxy” behavior into a shared helper (JSON/text/binary/SSE), migrates many /web/src/app/api/**/route.ts handlers to use it, and adds contract tests to prevent regressions. It also introduces/extends agent dashboard + agent-event vocabulary plumbing across the web UI and backend.
Changes:
- Add
web/src/app/api/_utils/backend-proxy.tsand migrate many Next route handlers to useproxyJson/proxyText/proxyBinary/proxyStream. - Remove dead runbook proxy routes and add contract tests (
tests/unit/test_next_backend_proxy_contracts.py,web/src/app/api/_utils/backend-proxy.test.ts). - Add agent-events store/parsers/UI components and backend codex delegation event helpers/constants.
Reviewed changes
Copilot reviewed 100 out of 101 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| web/vitest.config.ts | Configure jsdom env for component tests via environmentMatchGlobs. |
| web/src/lib/agent-events/useAgentEvents.ts | Client SSE hook for consuming backend /api/events/stream into Zustand. |
| web/src/lib/agent-events/types.ts | Agent event + Codex delegation display types. |
| web/src/lib/agent-events/store.ts | Zustand store for agent events, file touches, codex delegations, kanban tasks. |
| web/src/lib/agent-events/parsers.ts | Parse raw SSE envelopes into display/store entries. |
| web/src/components/layout/Sidebar.tsx | Add sidebar entry for Agent Dashboard. |
| web/src/components/agent-events/ToolCallTimeline.tsx | UI for tool call timeline panel. |
| web/src/components/agent-events/AgentStatusPanel.tsx | UI for agent status grid + connection indicator. |
| web/src/components/agent-events/ActivityFeed.tsx | UI for activity feed panel. |
| web/src/components/agent-dashboard/TasksPanel.tsx | Left rail panel (agents + runs selector). |
| web/src/components/agent-dashboard/KanbanBoard.tsx | Kanban board UI for agent tasks by status. |
| web/src/components/agent-dashboard/KanbanBoard.test.tsx | Component tests for KanbanBoard rendering/helpers. |
| web/src/components/agent-dashboard/InlineDiffPanel.tsx | Diff viewer wrapper panel for a selected file change. |
| web/src/components/agent-dashboard/FileListPanel.tsx | File list panel + navigation into InlineDiffPanel. |
| web/src/app/api/studio/status/route.ts | Migrate route to shared proxyJson. |
| web/src/app/api/studio/cwd/route.ts | Migrate route to shared proxyJson. |
| web/src/app/api/studio/chat/route.ts | Migrate SSE route to shared proxyStream with auth. |
| web/src/app/api/sandbox/status/route.ts | Migrate route to shared proxyJson. |
| web/src/app/api/runbook/snapshots/route.ts | Migrate route to shared proxyJson. |
| web/src/app/api/runbook/snapshots/[snapshotId]/route.ts | Migrate route to shared proxyJson. |
| web/src/app/api/runbook/smoke/route.ts | Remove dead proxy route. |
| web/src/app/api/runbook/runs/[runId]/route.ts | Remove dead proxy route. |
| web/src/app/api/runbook/revert/route.ts | Migrate route to shared proxyJson. |
| web/src/app/api/runbook/revert-project/route.ts | Migrate route to shared proxyJson (auth). |
| web/src/app/api/runbook/revert-hunks/route.ts | Migrate route to shared proxyJson. |
| web/src/app/api/runbook/project-dir/prepare/route.ts | Migrate route to shared proxyJson. |
| web/src/app/api/runbook/files/route.ts | Migrate route to shared proxyJson (query passthrough). |
| web/src/app/api/runbook/file/route.ts | Migrate GET/POST to shared proxyJson. |
| web/src/app/api/runbook/diff/route.ts | Migrate route to shared proxyJson (query passthrough). |
| web/src/app/api/runbook/delete/route.ts | Migrate route to shared proxyJson (auth). |
| web/src/app/api/runbook/changes/route.ts | Migrate route to shared proxyJson (query passthrough). |
| web/src/app/api/runbook/allowed-dirs/route.ts | Migrate GET/POST to shared proxyJson. |
| web/src/app/api/research/repro/context/route.ts | Use shared proxy stream helper for SSE generation. |
| web/src/app/api/research/paperscool/sessions/[sessionId]/route.ts | Migrate route to shared proxyText. |
| web/src/app/api/research/paperscool/daily/route.ts | Migrate to proxyStream with JSON fallback passthrough. |
| web/src/app/api/research/paperscool/analyze/route.ts | Migrate to proxyStream. |
| web/src/app/api/research/_base.ts | Replace local proxy helper with wrapper around shared proxy (auth=true). |
| web/src/app/api/papers/export/route.ts | Migrate binary download to shared proxyBinary. |
| web/src/app/api/newsletter/unsubscribe/[token]/route.ts | Migrate HTML passthrough to shared proxyText with custom error handler. |
| web/src/app/api/gen-code/route.ts | Migrate to shared proxyStream (auth). |
| web/src/app/api/auth/reset-password/route.ts | Migrate to shared proxyJson. |
| web/src/app/api/auth/register/route.ts | Migrate to shared proxyJson. |
| web/src/app/api/auth/me/route.ts | Migrate to shared proxyJson (auth) for GET/PATCH/DELETE. |
| web/src/app/api/auth/me/change-password/route.ts | Migrate to shared proxyJson (auth). |
| web/src/app/api/auth/login-check/route.ts | Migrate to shared proxyJson with onError fallback. |
| web/src/app/api/auth/forgot-password/route.ts | Migrate to shared proxyJson. |
| web/src/app/api/_utils/backend-proxy.ts | New shared proxy implementation (JSON/text/binary/SSE). |
| web/src/app/api/_utils/backend-proxy.test.ts | Unit tests for proxy helper behavior. |
| web/src/app/api/_utils/auth-headers.ts | Expand base URL env fallback chain. |
| web/src/app/agent-events/page.tsx | Agent events debug page mounting SSE hook + panels. |
| web/src/app/agent-dashboard/page.tsx | Agent dashboard page mounting SSE hook + SplitPanels layout. |
| web/package.json | Add testing deps + reorder deps (testing-library, jsdom, next-auth, radix popover). |
| tests/unit/test_next_backend_proxy_contracts.py | Contract tests ensuring Next route handlers don’t regress to direct fetch/env base URLs. |
| tests/unit/test_codex_overflow.py | Unit tests for codex overflow env-var stub. |
| tests/unit/test_agent_events_vocab.py | Extend EventType vocabulary tests incl. codex constants. |
| tests/unit/test_agent_board_codex_events.py | Unit tests for codex delegation event emission helper. |
| src/paperbot/repro/orchestrator.py | Add _should_overflow_to_codex() env-var stub. |
| src/paperbot/mcp/tools/_audit.py | Use EventType constants for tool result/error envelope types. |
| src/paperbot/application/collaboration/message_schema.py | Introduce EventType constants incl. codex delegation types. |
| src/paperbot/application/collaboration/agent_events.py | New helper module for lifecycle + tool-call event envelope creation. |
| src/paperbot/api/routes/agent_board.py | Add codex event emission helper + emit at dispatch/accept/complete/fail points. |
| .planning/phases/10-agent-board-codex-bridge/10-VALIDATION.md | Phase validation docs update. |
| .planning/phases/10-agent-board-codex-bridge/10-03-PLAN.md | Plan doc for dashboard integration/codex worker. |
| .planning/phases/10-agent-board-codex-bridge/10-02-SUMMARY.md | Summary doc for phase outputs. |
| .planning/phases/10-agent-board-codex-bridge/10-01-SUMMARY.md | Summary doc for backend codex event work. |
| .planning/phases/10-agent-board-codex-bridge/10-01-PLAN.md | Plan doc for codex event emission/constants. |
| .planning/phases/09-three-panel-dashboard/09-VERIFICATION.md | Phase verification docs update. |
| .planning/phases/09-three-panel-dashboard/09-VALIDATION.md | Phase validation docs update. |
| .planning/phases/09-three-panel-dashboard/09-02-SUMMARY.md | Summary doc update. |
| .planning/phases/09-three-panel-dashboard/09-01-SUMMARY.md | Summary doc update. |
| .planning/phases/08-agent-event-vocabulary/08-VERIFICATION.md | Phase verification docs update. |
| .planning/phases/08-agent-event-vocabulary/08-VALIDATION.md | Phase validation docs update. |
| .planning/phases/08-agent-event-vocabulary/08-02-SUMMARY.md | Summary doc update. |
| .planning/phases/08-agent-event-vocabulary/08-01-SUMMARY.md | Summary doc update. |
| .planning/phases/08-agent-event-vocabulary/08-01-PLAN.md | Plan doc update. |
| .planning/codebase/STACK.md | New/updated stack documentation. |
| .planning/codebase/CONVENTIONS.md | New/updated conventions documentation. |
| .planning/STATE.md | Update project state tracking. |
| .planning/REQUIREMENTS.md | Update requirements tracking/completion state. |
| .planning/PROJECT.md | Update project scope/roadmap narrative. |
Files not reviewed (1)
- web/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Comment on lines
+13
to
+17
| const abortRef = useRef<AbortController | null>(null) | ||
|
|
||
| useEffect(() => { | ||
| const controller = new AbortController() | ||
| abortRef.current = controller |
Comment on lines
+160
to
+168
| try { | ||
| return await fetch(upstreamUrl, { | ||
| ...init, | ||
| method, | ||
| headers: await resolveHeaders(req, method, options), | ||
| body: await resolveBody(req, method), | ||
| cache: options.cache, | ||
| signal: controller?.signal, | ||
| } as RequestInit & { dispatcher?: Agent }) |
Comment on lines
+43
to
+47
| @@ -43,6 +44,7 @@ const routes = [ | |||
| { label: "Papers", icon: FileText, href: "/papers" }, | |||
| { label: "Workflows", icon: Workflow, href: "/workflows" }, | |||
| { label: "DeepCode Studio", icon: Code2, href: "/studio" }, | |||
| { label: "Agent Dashboard", icon: Monitor, href: "/agent-dashboard" }, | |||
Comment on lines
+59
to
+70
| def _get_event_log_from_container(): | ||
| """Lazily retrieve the event_log from the DI Container. | ||
|
|
||
| Lazy import inside function body prevents circular imports at module load | ||
| time. Returns None if the container is not configured or has no event_log. | ||
| """ | ||
| try: | ||
| from paperbot.core.di import Container # noqa: PLC0415 | ||
|
|
||
| return Container.instance().event_log | ||
| except Exception: | ||
| return None |
Comment on lines
+828
to
+833
| await _emit_codex_event( | ||
| "codex_dispatched", | ||
| task, | ||
| session, | ||
| {"assignee": task.assignee}, | ||
| ) |
Comment on lines
+19
to
+25
| async function connect() { | ||
| try { | ||
| const res = await fetch(`${BACKEND_URL}/api/events/stream`, { | ||
| signal: controller.signal, | ||
| headers: { Accept: "text/event-stream" }, | ||
| }) | ||
| if (!res.ok || !res.body) return |
Comment on lines
+47
to
+54
| } catch (err) { | ||
| if ((err as Error)?.name !== "AbortError") { | ||
| console.warn("[useAgentEvents] disconnected, will retry in 3s", err) | ||
| setTimeout(connect, 3000) | ||
| } | ||
| } finally { | ||
| setConnected(false) | ||
| } |
jerry609
force-pushed
the
refactor/pr1-next-api-proxy
branch
from
f899732 to
0450c4d
Compare
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.planning/REQUIREMENTS.md (1)
259-297:⚠️ Potential issue | 🟡 MinorTraceability table is split, so v1.2 rows are detached from the table structure.
ADAPT-01throughDOMAIN-02appear after the Coverage bullet list, which breaks table continuity. Move those rows into the main traceability table (or start a new table with headers) so the mapping stays readable and machine-parseable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.planning/REQUIREMENTS.md around lines 259 - 297, The traceability rows for ADAPT-01 through DOMAIN-02 are outside the main table and break parsing; move all those rows back into the traceability table structure (or create a new markdown table with headers) so they are contiguous with the rest of the table; specifically relocate the block containing ADAPT-01 ... DOMAIN-02 into the main table body (or replace it with a properly formatted table header + rows) ensuring each requirement ID (e.g., ADAPT-01, MONIT-01, CTRL-01, SESS-01, VIS-01, DOMAIN-01) is a table row with the same columns as the existing traceability table for machine-parseability..planning/STATE.md (1)
43-72:⚠️ Potential issue | 🟡 MinorPerformance metrics section is internally inconsistent and partially malformed.
Line 44 conflicts with Line 13 (
6vs15completed plans), and Lines 63-72 are table-like rows without a valid table header context.Proposed cleanup
**Velocity:** -- Total plans completed: 6 +- Total plans completed: 15 - Average duration: 6 min - Total execution time: 0.6 hours **Recent Trend:** - Last 3 plans: 3min, 2min, 3min - Trend: Stable -| Phase 06-agent-skills P01 | 3 | 2 tasks | 5 files | -| Phase 07-eventbus-sse-foundation P01 | 3 | 2 tasks | 3 files | + +| Recent Phase | Duration | Tasks | Files | +|--------------|----------|-------|-------| +| Phase 06-agent-skills P01 | 3min | 2 tasks | 5 files | +| Phase 07-eventbus-sse-foundation P01 | 3min | 2 tasks | 3 files | ...🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.planning/STATE.md around lines 43 - 72, The "Velocity:" and "By Phase:" sections are inconsistent and the trailing rows under "Recent Trend:" are malformed table entries; update the "Velocity:" totals to match the sum of plans in "By Phase:" (or vice versa) and normalize the phase rows into a single valid Markdown table under the "By Phase:" header, remove or move the misplaced phase rows from the "Recent Trend:" section, and ensure "Recent Trend:" contains only simple bullet metrics (e.g., last N plan durations and trend). Specifically edit the "Velocity:" header, the "By Phase:" table rows (the entries like "02", "03-remaining-mcp-tools P01", etc.), and the stray rows currently at lines 63-72 so all counts (Total plans, Avg duration, per-phase plan counts) sum consistently.
🟡 Minor comments (13)
tests/unit/test_agent_board_codex_events.py-67-68 (1)
67-68:⚠️ Potential issue | 🟡 MinorRemove unused function
_fake_container_instance.This function is defined but never called. The test uses
monkeypatch.setattrdirectly with a lambda instead.🧹 Proposed fix
- def _fake_container_instance(): - return fake_container - monkeypatch.setattr( "paperbot.api.routes.agent_board._get_event_log_from_container", lambda: fake_log, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/test_agent_board_codex_events.py` around lines 67 - 68, Remove the unused helper function _fake_container_instance from the test file; it is defined but never called because the test uses monkeypatch.setattr with a lambda instead. Delete the _fake_container_instance function definition and any unused imports/refs tied only to it so the test remains clean and unchanged, and ensure monkeypatch.setattr usage (the lambda) remains as-is..planning/ROADMAP.md-174-179 (1)
174-179:⚠️ Potential issue | 🟡 MinorRoadmap status data is internally inconsistent.
Phase 9 is shown as complete (
2/2) in Progress, but its plan items are still unchecked, and Phase 8/10 progress rows appear column-shifted (plan counts ending up under Milestone). Please normalize these entries so phase tracking is unambiguous.Also applies to: 414-416
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.planning/ROADMAP.md around lines 174 - 179, The roadmap's phase rows have misaligned/mismatched status and checkbox items causing Phase 9 to show complete while plan checkboxes remain unchecked and Phase 8/10 counts are column-shifted; fix the table entries under the "Plans:" block so counts, progress (e.g., "2/2"), milestone and checkbox list are consistent: update the Phase 9 progress cell to reflect the unchecked items (e.g., "0/2" or mark the checkboxes checked to match "2/2"), and realign the numeric plan counts for Phase 8 and Phase 10 so they sit under the correct headers (Milestone vs Progress), ensuring the two plan lines ("09-01-PLAN.md" and "09-02-PLAN.md") accurately reflect checked state and that similar fixes are applied to lines 414-416 to normalize phase tracking..planning/codebase/INTEGRATIONS.md-146-147 (1)
146-147:⚠️ Potential issue | 🟡 MinorMarkdownlint MD049 warning should be resolved in this section.
This emphasis style is flagged by markdownlint (
Expected underscore; Actual asterisk). Please normalize to the repo’s configured emphasis style to keep docs lint-clean.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.planning/codebase/INTEGRATIONS.md around lines 146 - 147, Replace the asterisk-based emphasis in the indicated INTEGRATIONS.md line (the section mentioning "Pydantic EmailStr with `email-validator>=2.0.0`") with underscore-based emphasis to satisfy markdownlint MD049; locate the paragraph or list item containing "Pydantic EmailStr" and change any *italic* or **bold** asterisks to _italic_ or __bold__ respectively so the emphasis style matches the repo's configured underscore convention.web/src/components/agent-events/ToolCallTimeline.tsx-23-29 (1)
23-29:⚠️ Potential issue | 🟡 MinorFix invalid date handling and consolidate duplicate
formatTimestampfunction.The try/catch around
new Date(ts)won't trigger for invalid timestamps—JavaScript returns an Invalid Date object (non-fatal) rather than throwing. This causesd.toTimeString()to return the string"Invalid Date", which.slice(0, 8)reduces to"Invalid "in the UI, bypassing the fallback entirely.Additionally, this function is duplicated identically in
ActivityFeed.tsx. Extract to a shared utility module.Suggested fix
function formatTimestamp(ts: string): string { - try { - const d = new Date(ts) - return d.toTimeString().slice(0, 8) - } catch { - return ts.slice(0, 8) - } } + const d = new Date(ts) + if (Number.isNaN(d.getTime())) return ts.slice(0, 8) + return d.toTimeString().slice(0, 8) +}Move this function to a shared utils module (e.g.,
web/src/lib/agent-events/format.ts) and import in both components. Add tests for both valid and invalid timestamp inputs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/agent-events/ToolCallTimeline.tsx` around lines 23 - 29, The formatTimestamp function in ToolCallTimeline.tsx is incorrect because new Date(ts) does not throw for invalid inputs, so toTimeString() can yield "Invalid Date"; replace the implementation of formatTimestamp to explicitly check the Date validity (e.g., const d = new Date(ts); if (isNaN(d.getTime())) return fallback like ts.slice(0,8); else return d.toTimeString().slice(0,8)), then remove the duplicate implementation from ActivityFeed.tsx and extract this function into a shared utility (suggested module name: web/src/lib/agent-events/format.ts), import formatTimestamp into both ToolCallTimeline and ActivityFeed, and add unit tests for valid and invalid timestamp inputs.tests/unit/test_next_backend_proxy_contracts.py-10-15 (1)
10-15:⚠️ Potential issue | 🟡 MinorGuard against vacuous passes when no route files are discovered.
If
_route_files()returns empty, this contract test passes without validating anything.✅ Suggested hardening
def test_next_api_routes_use_shared_backend_proxy_helpers() -> None: offenders: list[str] = [] + route_files = _route_files() + assert route_files, f"No route.ts files found under {API_ROOT}" - for file_path in _route_files(): + for file_path in route_files: text = file_path.read_text(encoding="utf-8") rel = file_path.relative_to(ROOT)Also applies to: 18-31
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/test_next_backend_proxy_contracts.py` around lines 10 - 15, The test currently allows a vacuous pass when _route_files() returns an empty list; modify the test to guard against that by asserting that _route_files() is non-empty (or raising a clear AssertionError) before proceeding with contract validations so the test fails if no route files are discovered; locate the helper _route_files() and the test block(s) that iterate its result (in tests/unit/test_next_backend_proxy_contracts.py) and add a precondition check like assert routes, "no route files found: ensure API_ROOT and route.ts files are present" so missing discovery causes the test to fail rather than pass vacuously.web/src/components/agent-events/ActivityFeed.tsx-23-29 (1)
23-29:⚠️ Potential issue | 🟡 Minor
formatTimestamptry/catch doesn't catch invalid dates — UI shows "Invalid " text.JavaScript's
new Date(ts)doesn't throw on invalid input; it returns an Invalid Date object. The catch block never executes, andInvalid Date.slice(0, 8)renders as"Invalid "` in the UI.The same bug exists in
web/src/components/agent-events/ToolCallTimeline.tsx— both should be fixed consistently. Fix by checkingNumber.isNaN(d.getTime())instead of relying on try/catch. Add test coverage per the guideline that behavior changes should have tests.🛠️ Proposed fix
function formatTimestamp(ts: string): string { - try { - const d = new Date(ts) - return d.toTimeString().slice(0, 8) // HH:MM:SS - } catch { - return ts.slice(0, 8) - } + const d = new Date(ts) + if (Number.isNaN(d.getTime())) return ts.slice(0, 8) + return d.toISOString().slice(11, 19) // HH:MM:SS (UTC) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/agent-events/ActivityFeed.tsx` around lines 23 - 29, formatTimestamp currently assumes new Date(ts) throws on invalid input which it doesn't, so replace the try/catch with an explicit validity check: in formatTimestamp (web/src/components/agent-events/ActivityFeed.tsx) create the Date d from ts, then if Number.isNaN(d.getTime()) return a safe fallback like ts.slice(0,8) or an empty string, otherwise return d.toTimeString().slice(0,8); apply the same change in the corresponding formatter in web/src/components/agent-events/ToolCallTimeline.tsx; add unit tests covering valid ISO timestamps, invalid strings, and edge cases to ensure the fallback behavior is exercised..planning/phases/08-agent-event-vocabulary/08-RESEARCH.md-12-17 (1)
12-17:⚠️ Potential issue | 🟡 MinorFix malformed
Phase Requirementstable rendering.Line 17 is being parsed as part of the table, which causes the table-pipe/column-count lint failures and broken rendering.
Proposed markdown fix
| EVNT-01 | User can view a real-time scrolling activity feed showing agent events as they happen | useAgentEvents hook consuming /api/events/stream; feed component bound to Zustand activity list | | EVNT-02 | User can see each agent's lifecycle status (idle, working, completed, errored) at a glance | agent_started/agent_completed/agent_error typed events update per-agent status map in Zustand store | | EVNT-03 | User can view a structured tool call timeline showing tool name, arguments, result summary, and duration | tool_call/tool_result typed events with standardized payload shape; existing _audit.py already emits compatible data | + </phase_requirements>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.planning/phases/08-agent-event-vocabulary/08-RESEARCH.md around lines 12 - 17, The Phase Requirements markdown table (rows EVNT-01, EVNT-02, EVNT-03) is being parsed past its end; fix it by closing the table with a blank line (an empty newline) immediately after the final table row so the parser stops table-mode, or alternatively add a Markdown paragraph break (e.g., a blank line) before the following XML/phase tag. Edit the block that contains the table rows (the EVNT-XX entries) to ensure there is exactly one empty line after the table to prevent the next line from being treated as a table row..planning/phases/09-three-panel-dashboard/09-01-SUMMARY.md-126-127 (1)
126-127:⚠️ Potential issue | 🟡 MinorUnify emphasis style to satisfy markdownlint.
Lines 126-127 are flagged by MD049 (expected underscore emphasis style). Please align these footer lines with the configured style.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.planning/phases/09-three-panel-dashboard/09-01-SUMMARY.md around lines 126 - 127, The footer lines currently use asterisks for emphasis ("*Phase: 09-three-panel-dashboard*" and "*Completed: 2026-03-15*"), which triggers MD049; change them to use underscores for emphasis instead (e.g., _Phase: 09-three-panel-dashboard_ and _Completed: 2026-03-15_) so the emphasis style in the three-panel-dashboard summary matches the configured underscore style..planning/phases/10-agent-board-codex-bridge/10-01-PLAN.md-56-56 (1)
56-56:⚠️ Potential issue | 🟡 MinorResolve markdownlint warnings in this plan doc.
The current content still triggers MD037/MD046/MD040 at these lines (spaces inside emphasis markers, indented code block style, and a fence without language). Please normalize those sections so docs lint stays green.
Also applies to: 171-171, 222-222
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.planning/phases/10-agent-board-codex-bridge/10-01-PLAN.md at line 56, The plan doc has markdownlint violations (MD037, MD046, MD040); fix them by removing spaces inside emphasis markers (change "* text *" or "_ text _" to "*text*" or "_text_"), replace indented code blocks with fenced code blocks using backticks, and ensure every fenced code block includes a language hint (e.g., ```python) for the sections that currently show "Output: Python EventType constants, _emit_codex_event helper, _should_overflow_to_codex method, unit tests for all three." and the other flagged occurrences; normalize the three mentioned positions so there are no spaces inside emphasis markers, no indented code blocks, and all fences include a language..planning/phases/09-three-panel-dashboard/09-01-PLAN.md-183-183 (1)
183-183:⚠️ Potential issue | 🟡 MinorConvert indented snippets to fenced code blocks.
These lines are flagged by MD046. Use fenced blocks (with language where possible) to match the project markdown style and avoid lint noise.
Also applies to: 197-197, 306-306
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.planning/phases/09-three-panel-dashboard/09-01-PLAN.md at line 183, The indented code snippets under the "Store file tracking tests (append to store.test.ts):" section (and the similar occurrences flagged at the other two locations) must be converted from indented blocks to fenced code blocks using triple backticks and an appropriate language tag (e.g., ```ts or ```javascript) to satisfy MD046; find the snippet text under that heading and replace each indented snippet with a fenced block, and apply the same change to the other flagged snippets in the document (the ones reported by the linter) so all three are consistent..planning/phases/10-agent-board-codex-bridge/10-02-SUMMARY.md-56-56 (1)
56-56:⚠️ Potential issue | 🟡 MinorFix emphasis marker spacing on this line.
Line 56 is still flagged by markdownlint MD037 (
no-space-in-emphasis).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.planning/phases/10-agent-board-codex-bridge/10-02-SUMMARY.md at line 56, The MD037 error means there are spaces inside emphasis markers on the quoted line; edit the line containing "web/package.json, web/package-lock.json (added `@testing-library/react`, jsdom)" to remove spaces inside any asterisks/underscores or, better, mark filenames and packages as code using backticks (e.g. `web/package.json`, `web/package-lock.json`, `@testing-library/react`, `jsdom`) so there are no spaced emphasis markers remaining.web/src/lib/agent-events/parsers.ts-37-40 (1)
37-40:⚠️ Potential issue | 🟡 MinorInconsistent runtime guards for
payloadfields can leak[object Object]and bypass type validation.Payload is cast
as Record<string, unknown>(line 32), making field values untyped. UnguardedString()coercions throughout the file are unsafe. Additionally, thestatuscast at line 66 lacks runtime validation and can accept invalid values.The codebase already uses proper runtime guards elsewhere (e.g.,
typeof payload.error === "string"at line 22), so the unguarded cases should be hardened consistently.🛡️ Suggested hardening
+function asText(value: unknown, fallback = ""): string { + if (typeof value === "string") return value + if (typeof value === "number" || typeof value === "boolean") return String(value) + return fallback +} + ... - const tool = String(payload.tool ?? raw.stage ?? "unknown") + const tool = asText(payload.tool, asText(raw.stage, "unknown")) ... - const path = String( - (isExplicitFileChange - ? payload.path - : payload.arguments - ? (payload.arguments as Record<string, unknown>).path - : undefined) ?? "" - ) + const rawPath = isExplicitFileChange + ? payload.path + : (payload.arguments as Record<string, unknown> | undefined)?.path + const path = typeof rawPath === "string" ? rawPath : "" if (!path) return null ... - status: (payload.status as "created" | "modified") ?? "modified", + status: payload.status === "created" || payload.status === "modified" ? payload.status : "modified", ... - const task_id = String(payload.task_id ?? "") - const task_title = String(payload.task_title ?? "") - const assignee = String(payload.assignee ?? raw.agent_name ?? "") - const session_id = String(payload.session_id ?? "") + const task_id = asText(payload.task_id) + const task_title = asText(payload.task_title) + const assignee = asText(payload.assignee, asText(raw.agent_name)) + const session_id = asText(payload.session_id)Also applies to: 81–85, 93–98, 116, 155–159
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/lib/agent-events/parsers.ts` around lines 37 - 40, The code is using unsafe String() coercions on untyped payload fields (e.g., payload.detail, payload.tool, payload.result_summary) and an unchecked cast for status; replace these with runtime guards and safe formatting: check types like typeof payload.detail === "string" (or payload.detail == null) before using, for objects use JSON.stringify(payload.detail) or a fallback like "[object]" when not a string, and validate status against the expected union of values before accepting the cast. Update the branches that reference raw.agent_name, payload.detail, payload.tool, payload.result_summary and the status cast to perform these guards so object values do not become "[object Object]" and invalid status values are rejected or defaulted..planning/phases/08-agent-event-vocabulary/08-01-PLAN.md-112-118 (1)
112-118:⚠️ Potential issue | 🟡 MinorStrengthen the
_audit.pyregression test.This only proves the constants appear somewhere in the file. It still passes if
type="error"ortype="tool_result"survives beside the new constants, so the regression you're trying to prevent can slip through. Add explicit negative assertions for the rawtype=literals or inspect the relevant call site instead of doing a presence-only source check.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.planning/phases/08-agent-event-vocabulary/08-01-PLAN.md around lines 112 - 118, Update the test_audit_uses_constants unit test to not only assert that "EventType.TOOL_ERROR" and "EventType.TOOL_RESULT" appear in the _audit.py source, but also assert that the raw literals type="error" and type="tool_result" do NOT appear; alternatively, read the _audit.py AST or locate the specific call site (e.g., the audit/event creation call) and assert its type argument uses EventType.TOOL_ERROR / EventType.TOOL_RESULT rather than raw strings; keep the test synchronous, use direct file read or AST inspection, and keep the test named test_audit_uses_constants.
🧹 Nitpick comments (21)
.planning/codebase/STRUCTURE.md (1)
7-65: Add language specifier to fenced code block.The directory layout code block should have a language specifier for proper rendering and accessibility.
📝 Suggested fix
-``` +```text PaperBot/ ├── src/paperbot/ # Python backend source code🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.planning/codebase/STRUCTURE.md around lines 7 - 65, The fenced code block in .planning/codebase/STRUCTURE.md lacks a language specifier which can affect rendering; update the opening fence for the directory layout (the triple-backtick before the "PaperBot/" tree) to include a language tag such as text (i.e., change ``` to ```text) so the block is properly highlighted and accessible; ensure only the opening fence is modified and the rest of the ASCII tree (starting with "PaperBot/") remains unchanged..planning/research/STACK.md (1)
117-138: Add language specifier to architecture integration map.The architecture diagram should have a language specifier for consistency.
📝 Suggested fix
-``` +```text User Browser │ ├── SSE stream (EventSource) → FastAPI /api/agent/events → EventBus fan-out🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.planning/research/STACK.md around lines 117 - 138, Add a language/specifier label to the architecture integration map by inserting a short header or inline tag (e.g., "Language: text" or "Format: text") above or beside the ASCII diagram so it's clear what syntax/format the diagram uses; update the block containing "User Browser", "SSE stream (EventSource) → FastAPI /api/agent/events → EventBus fan-out" and the surrounding diagram lines (including "WebSocket (ws) → Next.js custom server" and "HTTP fetch (chat proxy) → Next.js API route") to include that language specifier consistently..planning/research/FEATURES.md (1)
75-125: Add language specifier to feature dependency graph.The feature dependency diagram should have a language specifier for consistency with other documentation.
📝 Suggested fix
-``` +```text Chat input / Task dispatch | +--requires--> Agent adapter layer (ClaudeCodeAdapter / CodexAdapter / OpenCodeAdapter)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.planning/research/FEATURES.md around lines 75 - 125, The ASCII feature dependency diagram starting with "Chat input / Task dispatch" is missing a language specifier on its fenced code block; update the opening fence to include a language (e.g., change ``` to ```text) so the block is consistently rendered and matches other docs; locate the fenced block containing "Chat input / Task dispatch" and replace the opening triple-backtick with ```text (keeping the same closing fence).web/src/app/api/auth/me/route.ts (1)
1-13: Missingruntimeexport for consistency.Other migrated routes (e.g.,
studio/cwd/route.ts,runbook/revert-project/route.ts) explicitly exportruntime = "nodejs". Consider adding this for consistency across the proxy routes:+export const runtime = "nodejs" + import { apiBaseUrl, proxyJson } from "@/app/api/_utils/backend-proxy"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/app/api/auth/me/route.ts` around lines 1 - 13, Add an explicit runtime export to match other migrated proxy routes: export const runtime = "nodejs". Place this top-level export in the same module that defines the GET, PATCH, and DELETE handlers so the runtime is applied to those functions (GET, PATCH, DELETE) and keeps behavior consistent with routes like studio/cwd/route.ts and runbook/revert-project/route.ts.tests/unit/test_codex_overflow.py (1)
10-56: Good TDD coverage for env var parsing.The test cases cover the primary truthy/falsy scenarios. The import-inside-test pattern is correct for capturing env var state at module load time.
Consider adding test cases for:
"false","no","off"(explicit falsy strings)- Case-insensitive variants like
"TRUE","Yes"if the implementation should handle themThis is optional if the implementation intentionally requires lowercase.
,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/test_codex_overflow.py` around lines 10 - 56, Add missing tests for explicit falsy strings and case variants for _should_overflow_to_codex: create tests using monkeypatch.setenv for "false", "no", and "off" asserting the function returns False, and add at least one case-insensitive check (e.g., "TRUE" returns True and "Yes"/"FALSE" behavior) to verify whether _should_overflow_to_codex handles casing; keep the import-inside-test pattern and mirror existing test style and names.web/src/components/agent-dashboard/InlineDiffPanel.tsx (1)
8-11: Consider marking props asReadonly.Per SonarCloud's suggestion, using
Readonly<InlineDiffPanelProps>ensures the component doesn't accidentally mutate its props—a TypeScript best practice for functional components.Proposed fix
-export function InlineDiffPanel({ entry, onBack }: InlineDiffPanelProps) { +export function InlineDiffPanel({ entry, onBack }: Readonly<InlineDiffPanelProps>) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/agent-dashboard/InlineDiffPanel.tsx` around lines 8 - 11, Wrap the component props in Readonly to prevent mutation: change usages that accept InlineDiffPanelProps (e.g., the InlineDiffPanel function/component signature and any prop-typed variables) to use Readonly<InlineDiffPanelProps> so the component receives an immutable props type; update the component declaration that references InlineDiffPanelProps to Readonly<InlineDiffPanelProps> and adjust any helper function/type annotations that consume the props accordingly.src/paperbot/repro/orchestrator.py (1)
56-57: Consider narrowing the exception catch.The bare
except Exceptionis flagged by Ruff (BLE001). Given thatos.getenv(),.strip(), and.lower()have well-defined exception behavior, this block is overly defensive. If you want to keep it for safety, catching a more specific set (e.g.,(TypeError, AttributeError)) would be cleaner.Proposed narrower exception handling
- except Exception: + except (TypeError, AttributeError): return False🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/paperbot/repro/orchestrator.py` around lines 56 - 57, The bare except in the block that wraps the os.getenv(...).strip().lower() call is too broad; narrow it to only the expected exceptions (e.g., catch (TypeError, AttributeError)) or remove the try/except and handle None explicitly before calling .strip()/.lower(); locate the try/except surrounding the os.getenv(...).strip().lower() expression in orchestrator.py and replace "except Exception:" with a specific tuple of exceptions (TypeError, AttributeError) or add an explicit None-check on the getenv result before calling .strip()/.lower().web/src/app/api/research/paperscool/sessions/[sessionId]/route.ts (1)
5-13: Consider usingproxyJsonfor JSON responses.The route uses
proxyTextwithresponseContentType: "application/json". If the backend returns structured JSON,proxyJsonwould be more semantically appropriate and likely handles JSON parsing/error cases more consistently with other routes.💡 Suggested change
-import { apiBaseUrl, proxyText } from "@/app/api/_utils/backend-proxy" +import { apiBaseUrl, proxyJson } from "@/app/api/_utils/backend-proxy" export async function GET(req: Request, ctx: { params: Promise<{ sessionId: string }> }) { const { sessionId } = await ctx.params - return proxyText( + return proxyJson( req, `${apiBaseUrl()}/api/research/paperscool/sessions/${encodeURIComponent(sessionId)}`, "GET", - { - cache: "no-store", - responseContentType: "application/json", - }, + { cache: "no-store" }, ) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/app/api/research/paperscool/sessions/`[sessionId]/route.ts around lines 5 - 13, The handler is calling proxyText for an endpoint that returns JSON; switch to proxyJson to better reflect/handle structured JSON responses. Replace the proxyText call with proxyJson (keeping the same URL built with apiBaseUrl() and encodeURIComponent(sessionId)) and remove or adjust the responseContentType option as proxyJson already handles JSON; ensure the HTTP method ("GET") and cache option ("no-store") are preserved so the call remains functionally identical.web/src/app/api/research/repro/context/route.ts (1)
1-2: Mixed import sources reflect intentional wrapper design but could be unified for consistency.This file imports
proxyJsonfrom"../../_base"(which wraps the backend function to addauth: trueby default) butproxyStreamfrom"@/app/api/_utils/backend-proxy". While this pattern is intentional—_basedoes not wrapproxyStream—unifying both imports from@/app/api/_utils/backend-proxyis valid for consistency. If unified, explicitly passauth: trueto theproxyJsoncall on line 8 to preserve the current behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/app/api/research/repro/context/route.ts` around lines 1 - 2, Unify the imports by importing both proxyJson and proxyStream from "@/app/api/_utils/backend-proxy" instead of proxyJson from "../../_base", and when calling proxyJson (the function currently invoked to call apiBaseUrl) explicitly pass { auth: true } to preserve the existing wrapped behavior; update the import statements and the proxyJson call site to include the auth option while leaving proxyStream usage unchanged.tests/unit/test_agent_board_codex_events.py (1)
33-46: Consider using simple dataclasses instead of MagicMock for test fakes.The coding guidelines recommend stub/fake classes over
unittest.mock. While the current usage is lightweight, a simple dataclass would be more explicit and align with the project's testing conventions.♻️ Alternative using dataclass
+from dataclasses import dataclass + +@dataclass +class FakeTask: + id: str = "task-abc" + title: str = "Implement model" + assignee: str = "codex-a1b2" + +@dataclass +class FakeSession: + session_id: str = "board-test-session" -def _make_task(task_id: str = "task-abc", title: str = "Implement model", assignee: str = "codex-a1b2"): - """Return a minimal AgentTask-like object without importing heavy deps.""" - task = MagicMock() - task.id = task_id - task.title = title - task.assignee = assignee - return task +def _make_task(task_id: str = "task-abc", title: str = "Implement model", assignee: str = "codex-a1b2"): + """Return a minimal AgentTask-like object without importing heavy deps.""" + return FakeTask(id=task_id, title=title, assignee=assignee) -def _make_session(session_id: str = "board-test-session"): - """Return a minimal BoardSession-like object.""" - session = MagicMock() - session.session_id = session_id - return session +def _make_session(session_id: str = "board-test-session"): + """Return a minimal BoardSession-like object.""" + return FakeSession(session_id=session_id)Based on learnings: "Use stub/fake classes (e.g.,
_FakeLLMService) rather thanunittest.mockfor test doubles in Python tests"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/test_agent_board_codex_events.py` around lines 33 - 46, Replace the MagicMock-based test fakes with simple dataclasses: create lightweight dataclasses (e.g., AgentTaskFake with fields id, title, assignee and BoardSessionFake with field session_id) and update the helper functions _make_task and _make_session to return instances of those dataclasses instead of MagicMock; ensure field names match the expectations of the tests/code that consumes these objects (id, title, assignee for the AgentTask-like object and session_id for the BoardSession-like object) so no other test changes are needed.web/src/lib/agent-events/store.ts (1)
81-86: Eviction relies onObject.keys()insertion order.The eviction logic assumes
keys[0]is the oldest run. This works because:
- Modern JS guarantees string key order matches insertion order
- Entries are only added, never re-inserted for existing run_ids
Consider adding a brief comment clarifying this assumption for future maintainers.
📝 Suggested clarifying comment
// Evict oldest run_id when exceeding 20 runs + // Object.keys() preserves insertion order for string keys, + // so keys[0] is the earliest-added run_id const keys = Object.keys(updated) if (keys.length > 20) { delete updated[keys[0]] }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/lib/agent-events/store.ts` around lines 81 - 86, Eviction uses Object.keys(updated)[0] to remove the oldest run but relies on JavaScript’s insertion-order guarantee for string keys and the fact that run_ids are only ever added (never re-inserted); add a concise comment next to the eviction block (around the updated/keys/filesTouched logic) stating this assumption and why it's safe (JS preserves insertion order for string keys and we don't reinsert existing run_ids), so future maintainers understand why keys[0] is the oldest and avoid accidental reordering changes.web/src/components/agent-dashboard/TasksPanel.tsx (1)
13-23: Consider memoizing the derived runs list.The runs derivation logic runs on every render. Since
feedcan contain up to 200 items (perFEED_MAXin store), consider wrapping this inuseMemoto avoid recalculating on unrelated re-renders.♻️ Proposed memoization
+import { useMemo } from "react" import { useAgentEventStore } from "@/lib/agent-events/store" ... - // Derive unique run_ids from feed, most recent first, limit 20 - const runs: { run_id: string; agent_name: string }[] = [] - const seen = new Set<string>() - for (const item of feed) { - const run_id = String(item.raw.run_id ?? "") - if (run_id && !seen.has(run_id)) { - seen.add(run_id) - runs.push({ run_id, agent_name: item.agent_name }) - if (runs.length >= 20) break - } - } + // Derive unique run_ids from feed, most recent first, limit 20 + const runs = useMemo(() => { + const result: { run_id: string; agent_name: string }[] = [] + const seen = new Set<string>() + for (const item of feed) { + const run_id = String(item.raw.run_id ?? "") + if (run_id && !seen.has(run_id)) { + seen.add(run_id) + result.push({ run_id, agent_name: item.agent_name }) + if (result.length >= 20) break + } + } + return result + }, [feed])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/agent-dashboard/TasksPanel.tsx` around lines 13 - 23, The runs derivation in the TasksPanel component recomputes on every render; wrap the logic that builds runs (the seen Set, run_id extraction, and runs.push) in a useMemo so it only recalculates when feed changes. Import useMemo from React if needed, replace the current for-loop with a const runs = useMemo(() => { /* existing loop creating runs array using seen and run_id, limit 20 */; return runs; }, [feed]) so the memo depends on feed and preserves the same run objects/shape.web/src/components/agent-dashboard/KanbanBoard.test.tsx (2)
62-69: The count-badge assertion is too broad.Asserting global text
"2"is prone to false positives. Prefer asserting the badge within the intended column header.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/agent-dashboard/KanbanBoard.test.tsx` around lines 62 - 69, The test "column header shows task count badge" asserts a global "2" which is too broad; update the test for KanbanBoard to scope the assertion to the specific column header DOM node: locate the column header element for status "planning" (e.g., findHeading/getByRole or getByText for the column title) and then use testing-library's within(headerElement).getByText("2") or expect(within(headerElement).queryByText("2")).toBeInTheDocument() so the badge count is asserted only inside that column header.
32-54: Column-placement tests are not actually validating column placement.These assertions only prove the task title exists somewhere on the board, so they can still pass if a task is rendered in the wrong column. Scope assertions to the specific column container (e.g., via
within(...)on the target column).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/agent-dashboard/KanbanBoard.test.tsx` around lines 32 - 54, The tests for KanbanBoard (tests that create tasks via makeTask and render <KanbanBoard />) currently assert only that a title exists on the page; change each test to scope the assertion to the specific column container by locating the column (e.g., find the "Planned", "In Progress", "Done", or "Blocked" column using screen.getByRole/getByText or a test id on the column) and then use within(column).getByText("...") instead of screen.getAllByText(...).length checks so each test verifies the task appears in the correct column.src/paperbot/api/routes/agent_board.py (2)
828-833: UseEventTypeconstants instead of raw"codex_*"strings.Lines 829, 2449, 2503, and 2537 pass raw literals even though this flow is tied to the shared event vocabulary. Using constants avoids typo regressions and keeps backend/frontend contracts aligned.
Also applies to: 2448-2453, 2502-2511, 2536-2545
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/paperbot/api/routes/agent_board.py` around lines 828 - 833, Replace raw string event names with the shared EventType constants: in calls to _emit_codex_event (and other emit sites) swap the literal "codex_dispatched" (and other "codex_*" literals) for the corresponding EventType member (e.g., EventType.codex_dispatched or the project's canonical EventType name). Update all occurrences that pass raw "codex_*" strings (including the other emit calls around the same flow) so they import and use EventType instead of hard-coded literals to keep the shared event vocabulary consistent.
73-78: Remove the unnecessaryasyncwrapper around_emit_codex_event.The function is declared as
asyncat line 73 but never awaits, adding unnecessary coroutine overhead and forcingawaitat every call site without benefit. Convert to a synchronous function.Proposed refactor
-async def _emit_codex_event( +def _emit_codex_event( event_type: str, task, session, extra: dict, ) -> None: @@ - await _emit_codex_event( + _emit_codex_event( "codex_dispatched", task, session, {"assignee": task.assignee}, ) @@ - await _emit_codex_event( + _emit_codex_event( "codex_dispatched", task, session, {"assignee": task.assignee}, ) @@ - await _emit_codex_event( + _emit_codex_event( "codex_accepted", task, session, {"assignee": task.assignee, "model": "codex"}, ) @@ - await _emit_codex_event( + _emit_codex_event( "codex_failed", task, session, { "assignee": task.assignee, "reason_code": result.diagnostics.get("reason_code", "unknown"), "error": str(result.error or ""), }, ) @@ - await _emit_codex_event( + _emit_codex_event( "codex_completed", task, session, { "assignee": task.assignee, "files_generated": task.generated_files, "output_preview": (result.output or "")[:200], }, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/paperbot/api/routes/agent_board.py` around lines 73 - 78, _emit_codex_event is declared async but contains no awaits, so convert it to a synchronous function: remove the async keyword and keep signature as def _emit_codex_event(event_type: str, task, session, extra: dict) -> None, adjust its return typing if needed, and update all call sites that currently use await _emit_codex_event(...) to call it directly (drop await); ensure any imports or annotations expecting a coroutine are updated accordingly..planning/research/ARCHITECTURE.md (1)
13-13: Add language tags to fenced blocks for lint/tooling consistency.This file has repeated unlabeled fences; labeling them (
text,python, etc.) will remove MD040 noise and improve rendering in editors.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.planning/research/ARCHITECTURE.md at line 13, The markdown file contains unlabeled fenced code blocks (``` ... ```) causing MD040 lint noise; update each unlabeled fence in .planning/research/ARCHITECTURE.md to include an appropriate language tag (e.g., ```text, ```python, ```json) matching the block content so tooling and linters correctly recognize the language and remove MD040 warnings..planning/phases/10-agent-board-codex-bridge/10-03-PLAN.md (1)
155-199: Use repo-relative paths in verification commands.Hardcoded local paths reduce portability and can break reproducibility across environments.
Proposed command updates
-<automated>test -f /home/master1/PaperBot/.claude/agents/codex-worker.md && head -3 /home/master1/PaperBot/.claude/agents/codex-worker.md && cd /home/master1/PaperBot/web && npm run build 2>&1 | tail -5</automated> +<automated>test -f .claude/agents/codex-worker.md && head -3 .claude/agents/codex-worker.md && cd web && npm run build 2>&1 | tail -5</automated> -<automated>cd /home/master1/PaperBot/web && npm run build 2>&1 | tail -3</automated> +<automated>cd web && npm run build 2>&1 | tail -3</automated> -cd /home/master1/PaperBot/web && npm run build 2>&1 | tail -5 +cd web && npm run build 2>&1 | tail -5 -cd /home/master1/PaperBot/web && npm test -- KanbanBoard agent-events 2>&1 | tail -10 +cd web && npm test -- KanbanBoard agent-events 2>&1 | tail -10🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.planning/phases/10-agent-board-codex-bridge/10-03-PLAN.md around lines 155 - 199, The verification commands in the .planning/phases/10-agent-board-codex-bridge/10-03-PLAN.md file use hardcoded absolute paths (e.g., /home/master1/PaperBot/web and /home/master1/PaperBot/.claude) which reduces portability; update the automated and verification command snippets (the lines under <automated> and the fenced bash block in <verification>) to use repo-relative paths (e.g., cd web && npm run build, test -f .claude/agents/codex-worker.md, etc.) so they run correctly in different environments; locate these commands in the PLAN.md content (search for the <automated> blocks and the fenced verification bash block) and replace absolute paths with relative ones while keeping the same checks and outputs..planning/phases/08-agent-event-vocabulary/08-RESEARCH.md (1)
69-69: Add language identifiers to fenced code blocks.Unlabeled fences reduce readability/tooling support and trigger repeated MD040 warnings.
Example fix pattern
-``` +```text src/paperbot/ ├── application/ │ └── collaboration/ ... -``` +```🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.planning/phases/08-agent-event-vocabulary/08-RESEARCH.md at line 69, The fenced code block at the shown snippet uses unlabeled backticks (```) which triggers MD040 and reduces tooling support; update the opening fence to include a language identifier (e.g., change ``` to ```text or ```bash as appropriate) for all unlabeled blocks in the document (look for occurrences of the triple-backtick fences such as the example around the project tree) so each fenced block is labeled and no longer triggers the MD040 warnings.web/src/lib/agent-events/types.ts (1)
20-21: Includetool_callin the typed tool-event surface.This file says it mirrors the backend event vocabulary, but
ToolCallEvent.typeonly allows terminal tool events. Any consumer that wants to type the pre-resulttool_callevent still has to fall back toAgentEventEnvelopeRaw, which defeats the point of exporting a dedicated tool event type.Suggested fix
export type ToolCallEvent = { - type: "tool_result" | "tool_error" + type: "tool_call" | "tool_result" | "tool_error" run_id: string trace_id: string agent_name: string🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/lib/agent-events/types.ts` around lines 20 - 21, ToolCallEvent currently only allows "tool_result" | "tool_error" and thus excludes the pre-result "tool_call" event; update the exported ToolCallEvent type so its type union includes "tool_call" (i.e., change the type literal union on ToolCallEvent.type to "tool_call" | "tool_result" | "tool_error") so consumers can statically type pre-result tool events without falling back to AgentEventEnvelopeRaw..planning/phases/10-agent-board-codex-bridge/10-RESEARCH.md (1)
77-77: Add language specifier to fenced code block.For better readability and to follow Markdown best practices, specify a language for the fenced code block. For file tree structures, use
textorplaintext.📝 Proposed fix
-``` +```text web/src/ ├── app/ │ └── agent-dashboard/As per coding guidelines flagged by markdownlint: fenced code blocks should have a language specified (MD040).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.planning/phases/10-agent-board-codex-bridge/10-RESEARCH.md at line 77, The fenced code block showing the file tree (the block containing "web/src/ ├── app/ └── agent-dashboard/") is missing a language specifier which triggers MD040; update that fenced code block to include a language tag such as "text" or "plaintext" (e.g., ```text) so markdownlint passes and the tree renders with the correct formatting.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c724e8aa-4396-4a83-b08e-6a526ce3ef65
⛔ Files ignored due to path filters (1)
web/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (100)
.planning/PROJECT.md.planning/REQUIREMENTS.md.planning/ROADMAP.md.planning/STATE.md.planning/codebase/ARCHITECTURE.md.planning/codebase/CONCERNS.md.planning/codebase/CONVENTIONS.md.planning/codebase/INTEGRATIONS.md.planning/codebase/STACK.md.planning/codebase/STRUCTURE.md.planning/codebase/TESTING.md.planning/phases/08-agent-event-vocabulary/08-01-PLAN.md.planning/phases/08-agent-event-vocabulary/08-01-SUMMARY.md.planning/phases/08-agent-event-vocabulary/08-02-PLAN.md.planning/phases/08-agent-event-vocabulary/08-02-SUMMARY.md.planning/phases/08-agent-event-vocabulary/08-RESEARCH.md.planning/phases/08-agent-event-vocabulary/08-VALIDATION.md.planning/phases/08-agent-event-vocabulary/08-VERIFICATION.md.planning/phases/09-three-panel-dashboard/09-01-PLAN.md.planning/phases/09-three-panel-dashboard/09-01-SUMMARY.md.planning/phases/09-three-panel-dashboard/09-02-PLAN.md.planning/phases/09-three-panel-dashboard/09-02-SUMMARY.md.planning/phases/09-three-panel-dashboard/09-RESEARCH.md.planning/phases/09-three-panel-dashboard/09-VALIDATION.md.planning/phases/09-three-panel-dashboard/09-VERIFICATION.md.planning/phases/10-agent-board-codex-bridge/10-01-PLAN.md.planning/phases/10-agent-board-codex-bridge/10-01-SUMMARY.md.planning/phases/10-agent-board-codex-bridge/10-02-PLAN.md.planning/phases/10-agent-board-codex-bridge/10-02-SUMMARY.md.planning/phases/10-agent-board-codex-bridge/10-03-PLAN.md.planning/phases/10-agent-board-codex-bridge/10-RESEARCH.md.planning/phases/10-agent-board-codex-bridge/10-VALIDATION.md.planning/research/ARCHITECTURE.md.planning/research/FEATURES.md.planning/research/PITFALLS.md.planning/research/STACK.md.planning/research/SUMMARY.mdsrc/paperbot/api/routes/agent_board.pysrc/paperbot/application/collaboration/agent_events.pysrc/paperbot/application/collaboration/message_schema.pysrc/paperbot/mcp/tools/_audit.pysrc/paperbot/repro/orchestrator.pytests/unit/test_agent_board_codex_events.pytests/unit/test_agent_events_vocab.pytests/unit/test_codex_overflow.pytests/unit/test_next_backend_proxy_contracts.pyweb/package.jsonweb/src/app/agent-dashboard/page.tsxweb/src/app/agent-events/page.tsxweb/src/app/api/_utils/auth-headers.tsweb/src/app/api/_utils/backend-proxy.test.tsweb/src/app/api/_utils/backend-proxy.tsweb/src/app/api/auth/forgot-password/route.tsweb/src/app/api/auth/login-check/route.tsweb/src/app/api/auth/me/change-password/route.tsweb/src/app/api/auth/me/route.tsweb/src/app/api/auth/register/route.tsweb/src/app/api/auth/reset-password/route.tsweb/src/app/api/gen-code/route.tsweb/src/app/api/newsletter/unsubscribe/[token]/route.tsweb/src/app/api/papers/export/route.tsweb/src/app/api/research/_base.tsweb/src/app/api/research/paperscool/analyze/route.tsweb/src/app/api/research/paperscool/daily/route.tsweb/src/app/api/research/paperscool/sessions/[sessionId]/route.tsweb/src/app/api/research/repro/context/route.tsweb/src/app/api/runbook/allowed-dirs/route.tsweb/src/app/api/runbook/changes/route.tsweb/src/app/api/runbook/delete/route.tsweb/src/app/api/runbook/diff/route.tsweb/src/app/api/runbook/file/route.tsweb/src/app/api/runbook/files/route.tsweb/src/app/api/runbook/project-dir/prepare/route.tsweb/src/app/api/runbook/revert-hunks/route.tsweb/src/app/api/runbook/revert-project/route.tsweb/src/app/api/runbook/revert/route.tsweb/src/app/api/runbook/runs/[runId]/route.tsweb/src/app/api/runbook/smoke/route.tsweb/src/app/api/runbook/snapshots/[snapshotId]/route.tsweb/src/app/api/runbook/snapshots/route.tsweb/src/app/api/sandbox/status/route.tsweb/src/app/api/studio/chat/route.tsweb/src/app/api/studio/cwd/route.tsweb/src/app/api/studio/status/route.tsweb/src/components/agent-dashboard/FileListPanel.tsxweb/src/components/agent-dashboard/InlineDiffPanel.tsxweb/src/components/agent-dashboard/KanbanBoard.test.tsxweb/src/components/agent-dashboard/KanbanBoard.tsxweb/src/components/agent-dashboard/TasksPanel.tsxweb/src/components/agent-events/ActivityFeed.tsxweb/src/components/agent-events/AgentStatusPanel.tsxweb/src/components/agent-events/ToolCallTimeline.tsxweb/src/components/layout/Sidebar.tsxweb/src/lib/agent-events/parsers.test.tsweb/src/lib/agent-events/parsers.tsweb/src/lib/agent-events/store.test.tsweb/src/lib/agent-events/store.tsweb/src/lib/agent-events/types.tsweb/src/lib/agent-events/useAgentEvents.tsweb/vitest.config.ts
💤 Files with no reviewable changes (2)
- web/src/app/api/runbook/smoke/route.ts
- web/src/app/api/runbook/runs/[runId]/route.ts
Comment on lines
+181
to
+183
| 1. **Create web/src/lib/agent-events/useAgentEvents.ts** — SSE consumer hook as specified in 08-RESEARCH.md Pattern 5. "use client" at top. Connect to `${BACKEND_URL}/api/events/stream` via fetch + readSSE(). For each incoming message: call parseActivityItem/parseAgentStatus/parseToolCall and dispatch to store. AbortController for cleanup. 3-second reconnect on error (not on AbortError). BACKEND_URL from `process.env.NEXT_PUBLIC_API_URL ?? "http://localhost:8000"`. | ||
|
|
||
| IMPORTANT: readSSE() yields SSEMessage objects, but the SSE endpoint sends raw AgentEventEnvelope dicts as JSON. The readSSE() parser returns whatever JSON.parse produces. Since the events endpoint sends `data: {envelope_dict}\n\n`, readSSE() will yield the parsed envelope dict directly. Cast `msg as AgentEventEnvelopeRaw` and check for `raw?.type` before dispatching. |
There was a problem hiding this comment.
Use the proxied /api/events/stream route here.
Pointing the browser hook at ${BACKEND_URL}/api/events/stream bypasses the same-origin Next API layer this refactor is trying to standardize. That reintroduces CORS/cookie/auth drift and contradicts the /api/events/stream contract described earlier in the same plan. Prefer a relative /api/events/stream unless there is a deliberate exception.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.planning/phases/08-agent-event-vocabulary/08-02-PLAN.md around lines 181 -
183, The hook useAgentEvents.ts should not call
`${BACKEND_URL}/api/events/stream`; update the fetch target to the relative path
"/api/events/stream" so the browser uses the Next API proxy and preserves
same-origin cookies/auth; keep the rest of the logic (readSSE(), casting msg as
AgentEventEnvelopeRaw and checking raw?.type, calling
parseActivityItem/parseAgentStatus/parseToolCall and dispatching to store, using
AbortController and 3s reconnect on non-AbortError) unchanged.
Comment on lines
+194
to
+195
| - Clicking a run button calls `setSelectedRunId(id)`. Clicking the same run again deselects (passes `null`) | ||
| - Highlight the selected run with `bg-accent` class |
There was a problem hiding this comment.
Clear selectedFile when the active run changes.
With this flow, once a file is open the right pane stays on InlineDiffPanel even after the user picks a different run, so the promised “file list filters to that run’s files” behavior never happens until they manually go back. Have the run-selection path clear selectedFile, or ignore a selected file that does not belong to the active run.
Also applies to: 201-205
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.planning/phases/09-three-panel-dashboard/09-02-PLAN.md around lines 194 -
195, When the active run changes via setSelectedRunId(id) ensure the selected
file state is reset: call setSelectedFile(null) (or equivalent) inside the
run-selection handler (the same place that toggles selected run) so the right
pane doesn't remain showing InlineDiffPanel for the previous run; alternatively
implement a check in the render path for InlineDiffPanel to ignore or clear
selectedFile if selectedFile.runId !== selectedRunId so a file that doesn't
belong to the active run is not shown. Update the handler and/or InlineDiffPanel
selection validation to enforce this behavior.
| | **Framework** | vitest 2.1.4 (frontend); pytest + pytest-asyncio (backend) | | ||
| | **Config file** | `web/vitest.config.ts` — environment: "node", alias: "@" → "./src" | | ||
| | **Quick run command** | `cd web && npm test -- KanbanBoard agent-events` | | ||
| | **Full suite command** | `cd web && npm test -- KanbanBoard agent-events && PYTHONPATH=src pytest tests/unit/test_agent_board_route.py tests/unit/test_codex_overflow.py -q` | |
There was a problem hiding this comment.
Run backend pytest from repo root, not inside web.
Line 23 and Line 31 chain cd web && ... && pytest ..., so pytest executes from web/. That can break tests/unit/... path resolution and PYTHONPATH=src targeting for backend tests.
Suggested fix
-| **Full suite command** | `cd web && npm test -- KanbanBoard agent-events && PYTHONPATH=src pytest tests/unit/test_agent_board_route.py tests/unit/test_codex_overflow.py -q` |
+| **Full suite command** | `(cd web && npm exec vitest run src/components/agent-dashboard/KanbanBoard.test.tsx src/lib/agent-events/parsers.test.ts) && PYTHONPATH=src pytest tests/unit/test_agent_board_route.py tests/unit/test_codex_overflow.py -q` |-- **After every plan wave:** Run `cd web && npm test -- KanbanBoard agent-events && PYTHONPATH=src pytest tests/unit/test_agent_board_route.py tests/unit/test_codex_overflow.py -q`
+- **After every plan wave:** Run `(cd web && npm exec vitest run src/components/agent-dashboard/KanbanBoard.test.tsx src/lib/agent-events/parsers.test.ts) && PYTHONPATH=src pytest tests/unit/test_agent_board_route.py tests/unit/test_codex_overflow.py -q`Also applies to: 31-31
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.planning/phases/10-agent-board-codex-bridge/10-VALIDATION.md at line 23,
The full-suite command currently chains "cd web && npm test -- KanbanBoard
agent-events && PYTHONPATH=src pytest tests/unit/..." which causes pytest to run
from web/; change it so npm test runs inside web but pytest runs from the repo
root, e.g. split the chain so "(cd web && npm test -- KanbanBoard agent-events)
&& PYTHONPATH=src pytest tests/unit/test_agent_board_route.py
tests/unit/test_codex_overflow.py -q" (ensure the command string labeled "Full
suite command" is updated accordingly).
web/package.json
Outdated
| "framer-motion": "^12.23.26", | ||
| "lucide-react": "^0.562.0", | ||
| "next": "16.1.0", | ||
| "next-auth": "5.0.0-beta.30", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
next-auth 5.0 stable release status 2026
💡 Result:
As of March 15, 2026, NextAuth/Auth.js v5 has not had a “stable” (non-beta) 5.0.0 release—it’s still distributed via the next-auth@beta tag (the official v5 migration/upgrade guide still tells you to install v5 using next-auth@beta). [1]
The latest stable release line remains v4 (e.g., 4.24.13, dated Oct 29, 2025), while the latest v5 published version is still a beta (e.g., 5.0.0-beta.30, dated Oct 27, 2025). [2][3]
Use of next-auth beta version requires production readiness assessment.
Version 5.0.0-beta.30 remains in beta as of March 2026 with no stable v5.0.0 release available. The stable release line is still v4 (latest: 4.24.13). Using a beta authentication library poses security and stability risks for production. Consider either:
- Migrating to NextAuth v4 for production stability, or
- Documenting acceptance of v5 beta risks and planning migration timeline before production deployment.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/package.json` at line 46, The package.json currently pins "next-auth":
"5.0.0-beta.30" which is a beta release; either revert to the stable v4 line or
explicitly document acceptance of beta risk: update the dependency entry
"next-auth" to a stable v4 version (e.g., "4.24.13" or a caret range like
"^4.24.13") in package.json to ensure production stability, or if you
intentionally keep v5 beta, add a clear note in the repo (README or
DEPLOYMENT.md) referencing "next-auth": "5.0.0-beta.30" and an approved
risk/mitigation plan and migration timeline before production rollout.
Comment on lines
+84
to
+102
| try { | ||
| const upstream = await fetchUpstream(req, upstreamUrl, method, options) | ||
| const body = await upstream.arrayBuffer() | ||
| const headers = new Headers(options.responseHeaders) | ||
| headers.set("Cache-Control", "no-cache") | ||
| headers.set( | ||
| "Content-Type", | ||
| upstream.headers.get("content-type") || "application/octet-stream", | ||
| ) | ||
|
|
||
| const contentDisposition = upstream.headers.get("content-disposition") | ||
| if (contentDisposition) { | ||
| headers.set("Content-Disposition", contentDisposition) | ||
| } | ||
|
|
||
| return new Response(body, { | ||
| status: upstream.status, | ||
| headers, | ||
| }) |
There was a problem hiding this comment.
Stream binary downloads instead of buffering the whole response.
arrayBuffer() forces the entire upstream payload into memory before the first byte is sent. That's a bad fit for export/download routes and also drops useful upstream metadata like length/range headers unless you copy them back manually.
Suggested fix
try {
const upstream = await fetchUpstream(req, upstreamUrl, method, options)
- const body = await upstream.arrayBuffer()
- const headers = new Headers(options.responseHeaders)
+ const headers = new Headers(upstream.headers)
+ headers.delete("content-length")
+ headers.delete("content-encoding")
+ headers.delete("connection")
+ headers.delete("transfer-encoding")
+ for (const [key, value] of new Headers(options.responseHeaders)) {
+ headers.set(key, value)
+ }
headers.set("Cache-Control", "no-cache")
- headers.set(
- "Content-Type",
- upstream.headers.get("content-type") || "application/octet-stream",
- )
-
- const contentDisposition = upstream.headers.get("content-disposition")
- if (contentDisposition) {
- headers.set("Content-Disposition", contentDisposition)
+ if (!headers.has("Content-Type")) {
+ headers.set("Content-Type", "application/octet-stream")
}
- return new Response(body, {
+ return new Response(upstream.body, {
status: upstream.status,
headers,
})📝 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
| try { | |
| const upstream = await fetchUpstream(req, upstreamUrl, method, options) | |
| const body = await upstream.arrayBuffer() | |
| const headers = new Headers(options.responseHeaders) | |
| headers.set("Cache-Control", "no-cache") | |
| headers.set( | |
| "Content-Type", | |
| upstream.headers.get("content-type") || "application/octet-stream", | |
| ) | |
| const contentDisposition = upstream.headers.get("content-disposition") | |
| if (contentDisposition) { | |
| headers.set("Content-Disposition", contentDisposition) | |
| } | |
| return new Response(body, { | |
| status: upstream.status, | |
| headers, | |
| }) | |
| try { | |
| const upstream = await fetchUpstream(req, upstreamUrl, method, options) | |
| const headers = new Headers(upstream.headers) | |
| headers.delete("content-length") | |
| headers.delete("content-encoding") | |
| headers.delete("connection") | |
| headers.delete("transfer-encoding") | |
| for (const [key, value] of new Headers(options.responseHeaders)) { | |
| headers.set(key, value) | |
| } | |
| headers.set("Cache-Control", "no-cache") | |
| if (!headers.has("Content-Type")) { | |
| headers.set("Content-Type", "application/octet-stream") | |
| } | |
| return new Response(upstream.body, { | |
| status: upstream.status, | |
| headers, | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/app/api/_utils/backend-proxy.ts` around lines 84 - 102, The handler
currently buffers the entire upstream response via upstream.arrayBuffer() which
uses excessive memory and drops streaming headers; change the return to stream
the upstream.body instead: use upstream.body as the Response body (or a
transformed ReadableStream if you need to inject behavior) and avoid
arrayBuffer(); preserve and copy upstream headers like content-length,
content-range, accept-ranges (in addition to content-disposition and
content-type) into the Headers built from options.responseHeaders, still
overriding Cache-Control to "no-cache"; update the code around
fetchUpstream/const upstream to return new Response(upstream.body, { status:
upstream.status, headers }) so downloads and range requests work and memory
usage stays low.
Comment on lines
+67
to
+103
| export function AgentStatusPanel({ compact = false }: { compact?: boolean }) { | ||
| const agentStatuses = useAgentEventStore((s) => s.agentStatuses) | ||
| const connected = useAgentEventStore((s) => s.connected) | ||
| const entries = Object.values(agentStatuses) | ||
|
|
||
| return ( | ||
| <div className="flex flex-col gap-2 p-3"> | ||
| {!compact && ( | ||
| <div className="flex items-center justify-between"> | ||
| <h3 className="text-sm font-semibold text-gray-200">Agent Status</h3> | ||
| <div className="flex items-center gap-1.5"> | ||
| {connected ? ( | ||
| <> | ||
| <Wifi size={12} className="text-green-400" /> | ||
| <span className="text-xs text-green-400">Connected</span> | ||
| </> | ||
| ) : ( | ||
| <> | ||
| <WifiOff size={12} className="text-amber-400" /> | ||
| <span className="text-xs text-amber-400">Connecting...</span> | ||
| </> | ||
| )} | ||
| </div> | ||
| </div> | ||
| )} | ||
| {entries.length === 0 ? ( | ||
| <div className="text-xs text-gray-500">No agents active</div> | ||
| ) : ( | ||
| <div className={compact ? "grid grid-cols-1 gap-2" : "grid grid-cols-2 gap-2 sm:grid-cols-3 lg:grid-cols-4"}> | ||
| {entries.map((entry) => ( | ||
| <AgentStatusBadge key={entry.agent_name} entry={entry} compact={compact} /> | ||
| ))} | ||
| </div> | ||
| )} | ||
| </div> | ||
| ) | ||
| } |
There was a problem hiding this comment.
Add tests for AgentStatusPanel rendering and state mapping.
This introduces meaningful UI behavior (status label/icon mapping, compact truncation, connection state, empty state), but no tests are included here to protect it from regressions.
As per coding guidelines If behavior changes, add or update tests.
🧰 Tools
🪛 GitHub Check: SonarCloud Code Analysis
[warning] 67-67: Mark the props of the component as read-only.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/components/agent-events/AgentStatusPanel.tsx` around lines 67 - 103,
Add unit tests for the AgentStatusPanel React component to cover rendering and
state mapping: mount AgentStatusPanel with mocked useAgentEventStore values to
assert (1) empty state shows "No agents active", (2) when entries exist it
renders an AgentStatusBadge per agent (keyed by agent_name) and grid classes
change when compact=true (compact truncation), (3) connection indicator shows
Wifi/WifiOff icon and "Connected"/"Connecting..." text based on connected
boolean, and (4) agent status-to-label/icon mapping is passed through to
AgentStatusBadge (mock or spy the component to verify props). Use the test
renderer/RTL and mock useAgentEventStore to return controlled agentStatuses and
connected states for each scenario.
Comment on lines
+1
to
+94
| "use client" | ||
|
|
||
| import { useAgentEventStore } from "@/lib/agent-events/store" | ||
| import type { ToolCallEntry } from "@/lib/agent-events/types" | ||
|
|
||
| function formatDuration(ms: number): string { | ||
| if (ms === 0) return "—" | ||
| if (ms >= 1000) return `${(ms / 1000).toFixed(1)}s` | ||
| return `${Math.round(ms)}ms` | ||
| } | ||
|
|
||
| function formatArgs(args: Record<string, unknown>): string { | ||
| const keys = Object.keys(args) | ||
| if (keys.length === 0) return "(no args)" | ||
| return keys.join(", ") | ||
| } | ||
|
|
||
| function truncate(text: string, max = 100): string { | ||
| if (text.length <= max) return text | ||
| return text.slice(0, max) + "…" | ||
| } | ||
|
|
||
| function formatTimestamp(ts: string): string { | ||
| try { | ||
| const d = new Date(ts) | ||
| return d.toTimeString().slice(0, 8) | ||
| } catch { | ||
| return ts.slice(0, 8) | ||
| } | ||
| } | ||
|
|
||
| function ToolCallRow({ entry }: { entry: ToolCallEntry }) { | ||
| const isError = entry.status === "error" | ||
|
|
||
| return ( | ||
| <li className="flex items-start gap-3 py-2 border-b border-gray-800 last:border-0"> | ||
| <div | ||
| className={`w-2 h-2 rounded-full mt-1.5 shrink-0 ${isError ? "bg-red-500" : "bg-green-500"}`} | ||
| /> | ||
| <div className="flex-1 min-w-0"> | ||
| <div className="flex items-center justify-between gap-2"> | ||
| <span className="text-sm font-semibold text-gray-100 truncate">{entry.tool}</span> | ||
| <div className="flex items-center gap-2 shrink-0"> | ||
| {isError && ( | ||
| <span className="text-xs px-1.5 py-0.5 rounded bg-red-950/60 text-red-400 border border-red-800/50"> | ||
| error | ||
| </span> | ||
| )} | ||
| <span className="text-xs text-gray-400 font-mono">{formatDuration(entry.duration_ms)}</span> | ||
| <span className="text-xs text-gray-600 font-mono">{formatTimestamp(entry.ts)}</span> | ||
| </div> | ||
| </div> | ||
| <div className="text-xs text-gray-500 mt-0.5"> | ||
| <span className="text-gray-600">args: </span> | ||
| <span>{formatArgs(entry.arguments)}</span> | ||
| </div> | ||
| {entry.result_summary && !isError && ( | ||
| <div className="text-xs text-gray-400 mt-0.5 truncate"> | ||
| {truncate(entry.result_summary)} | ||
| </div> | ||
| )} | ||
| {isError && entry.error && ( | ||
| <div className="text-xs text-red-400 mt-0.5 truncate">{entry.error}</div> | ||
| )} | ||
| </div> | ||
| </li> | ||
| ) | ||
| } | ||
|
|
||
| export function ToolCallTimeline() { | ||
| const toolCalls = useAgentEventStore((s) => s.toolCalls) | ||
|
|
||
| return ( | ||
| <div className="flex flex-col h-full"> | ||
| <div className="flex items-center justify-between px-3 py-2 border-b border-gray-700"> | ||
| <h3 className="text-sm font-semibold text-gray-200">Tool Calls</h3> | ||
| <span className="text-xs text-gray-500">{toolCalls.length} calls</span> | ||
| </div> | ||
| {toolCalls.length === 0 ? ( | ||
| <div className="flex items-center justify-center h-20 text-gray-500 text-sm"> | ||
| No tool calls yet | ||
| </div> | ||
| ) : ( | ||
| <div className="overflow-y-auto flex-1"> | ||
| <ul className="px-3 py-1"> | ||
| {toolCalls.map((entry) => ( | ||
| <ToolCallRow key={entry.id} entry={entry} /> | ||
| ))} | ||
| </ul> | ||
| </div> | ||
| )} | ||
| </div> | ||
| ) | ||
| } |
There was a problem hiding this comment.
Please add tests for this new component’s behavior branches.
I don’t see coverage here for duration formatting, invalid timestamp handling, args rendering, and success/error row variants. This should be locked down with a dedicated test file.
As per coding guidelines 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/agent-events/ToolCallTimeline.tsx` around lines 1 - 94,
Add unit tests for the ToolCallTimeline/ToolCallRow behaviors: write tests that
import ToolCallTimeline (or render ToolCallRow directly) and assert
formatDuration outputs for 0ms, <1000ms, and >=1000ms; that formatTimestamp
falls back gracefully for invalid ts values; that formatArgs renders "(no args)"
for empty objects and lists keys for non-empty args; and that UI shows the
success variant (result_summary visible, green dot) vs error variant (red dot,
error badge and error text shown). Target the functions formatDuration,
formatTimestamp, formatArgs, truncate and the ToolCallRow component so each
branch (zero duration, ms vs s, invalid timestamp, empty vs non-empty args,
success vs error) has a dedicated test assertion.
web/src/lib/agent-events/parsers.ts
Outdated
Comment on lines
+30
to
+67
| function deriveHumanSummary(raw: AgentEventEnvelopeRaw): string { | ||
| const t = raw.type ?? "" | ||
| const payload = (raw.payload ?? {}) as Record<string, unknown> | ||
|
|
||
| if (t === "agent_started") return `${raw.agent_name} started: ${raw.stage}` | ||
| if (t === "agent_working") return `${raw.agent_name} working on: ${raw.stage}` | ||
| if (t === "agent_completed") return `${raw.agent_name} completed: ${raw.stage}` | ||
| if (t === "agent_error") return `${raw.agent_name} error: ${String(payload.detail ?? "")}` | ||
| if (t === "tool_result" || t === "tool_error" || t === "tool_call") { | ||
| const tool = String(payload.tool ?? t) | ||
| return `Tool: ${tool} — ${String(payload.result_summary ?? "").slice(0, 80)}` | ||
| } | ||
| if (t === "codex_dispatched") { | ||
| const assignee = String(payload.assignee ?? raw.agent_name ?? "Codex") | ||
| const title = String(payload.task_title ?? "") | ||
| return `Task dispatched to ${assignee}: ${title}` | ||
| } | ||
| if (t === "codex_accepted") { | ||
| const title = String(payload.task_title ?? "") | ||
| return `Codex accepted task: ${title}` | ||
| } | ||
| if (t === "codex_completed") { | ||
| const title = String(payload.task_title ?? "") | ||
| const files = Array.isArray(payload.files_generated) ? payload.files_generated.length : 0 | ||
| return `Codex completed: ${title} (${files} files)` | ||
| } | ||
| if (t === "codex_failed") { | ||
| const title = String(payload.task_title ?? "") | ||
| const reason = String(payload.reason_code ?? "unknown") | ||
| return `Codex failed: ${title} (${reason})` | ||
| } | ||
| if (t === "job_start") return `Job started: ${raw.stage}` | ||
| if (t === "job_result") return `Job finished: ${raw.stage}` | ||
| if (t === "source_record") return `Source record: ${raw.workflow}/${raw.stage}` | ||
| if (t === "score_update") return `Score update from ${raw.agent_name}` | ||
| if (t === "insight") return `Insight from ${raw.agent_name}` | ||
| return `${t}: ${raw.agent_name ?? ""} / ${raw.stage ?? ""}` | ||
| } |
There was a problem hiding this comment.
deriveHumanSummary exceeds configured cognitive-complexity threshold.
This function is currently over the Sonar limit and is reported as a failing check. Refactor to a type→formatter map to keep the quality gate green.
♻️ Suggested refactor shape
+type SummaryFormatter = (
+ raw: AgentEventEnvelopeRaw,
+ payload: Record<string, unknown>
+) => string
+
+const SUMMARY_FORMATTERS: Record<string, SummaryFormatter> = {
+ agent_started: (raw) => `${raw.agent_name} started: ${raw.stage}`,
+ agent_working: (raw) => `${raw.agent_name} working on: ${raw.stage}`,
+ agent_completed: (raw) => `${raw.agent_name} completed: ${raw.stage}`,
+ agent_error: (raw, payload) => `${raw.agent_name} error: ${String(payload.detail ?? "")}`,
+ // ...codex/tool/job mappings...
+}
+
function deriveHumanSummary(raw: AgentEventEnvelopeRaw): string {
const t = raw.type ?? ""
const payload = (raw.payload ?? {}) as Record<string, unknown>
-
- if (t === "agent_started") return `${raw.agent_name} started: ${raw.stage}`
- if (t === "agent_working") return `${raw.agent_name} working on: ${raw.stage}`
- ...
+ const formatter = SUMMARY_FORMATTERS[t]
+ if (formatter) return formatter(raw, payload)
return `${t}: ${raw.agent_name ?? ""} / ${raw.stage ?? ""}`
}🧰 Tools
🪛 GitHub Check: SonarCloud Code Analysis
[warning] 48-48: 'payload.task_title ?? ""' will use Object's default stringification format ('[object Object]') when stringified.
[warning] 37-37: 'payload.detail ?? ""' will use Object's default stringification format ('[object Object]') when stringified.
[warning] 39-39: 'payload.tool ?? t' will use Object's default stringification format ('[object Object]') when stringified.
[warning] 40-40: 'payload.result_summary ?? ""' will use Object's default stringification format ('[object Object]') when stringified.
[warning] 58-58: 'payload.reason_code ?? "unknown"' will use Object's default stringification format ('[object Object]') when stringified.
[warning] 52-52: 'payload.task_title ?? ""' will use Object's default stringification format ('[object Object]') when stringified.
[failure] 30-30: Refactor this function to reduce its Cognitive Complexity from 16 to the 15 allowed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/lib/agent-events/parsers.ts` around lines 30 - 67, The
deriveHumanSummary function is too complex; replace the big if/else chain with a
map from event type string to formatter functions: create a Record<string, (raw:
AgentEventEnvelopeRaw, payload: Record<string,unknown>) => string> where each
key (e.g., "agent_started", "tool_result", "codex_completed", "codex_failed",
"job_start", "source_record", etc.) implements the corresponding logic currently
in deriveHumanSummary (use raw.agent_name, raw.stage, payload fields like
payload.tool, payload.result_summary, payload.task_title,
payload.files_generated, payload.reason_code, etc.), preserve existing behaviors
(slice result_summary to 80 chars for tool_* types, default assignee fallback
for codex_dispatched), and use a default formatter for unknown types that
returns `${t}: ${raw.agent_name ?? ""} / ${raw.stage ?? ""}`; then have
deriveHumanSummary simply look up the formatter by t (with payload typed as
Record<string, unknown>) and return the result.
| import { parseActivityItem, parseAgentStatus, parseToolCall, parseFileTouched, parseCodexDelegation } from "./parsers" | ||
| import type { AgentEventEnvelopeRaw } from "./types" | ||
|
|
||
| const BACKEND_URL = process.env.NEXT_PUBLIC_API_URL ?? "http://localhost:8000" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n web/src/lib/agent-events/useAgentEvents.ts | head -70Repository: jerry609/PaperBot
Length of output: 2592
🏁 Script executed:
find web/src -name "*.ts" -o -name "*.tsx" | head -20Repository: jerry609/PaperBot
Length of output: 903
🏁 Script executed:
rg "NEXT_PUBLIC_API_URL" web/src --type ts --type tsx -B 2 -A 2Repository: jerry609/PaperBot
Length of output: 88
🏁 Script executed:
rg "/api/" web/src --type ts --type tsx -l | head -10Repository: jerry609/PaperBot
Length of output: 88
🏁 Script executed:
cat web/src/app/api/_utils/backend-proxy.tsRepository: jerry609/PaperBot
Length of output: 6600
🏁 Script executed:
cat web/src/app/api/_utils/auth-headers.tsRepository: jerry609/PaperBot
Length of output: 1157
🏁 Script executed:
rg "fetch.*api/" web/src --type ts -B 2 -A 2 | head -50Repository: jerry609/PaperBot
Length of output: 3783
🏁 Script executed:
find web/src -path "*/test*" -o -path "*.test.*" | head -10Repository: jerry609/PaperBot
Length of output: 529
🏁 Script executed:
find web/src/app/api -name "*event*" -o -name "*stream*"Repository: jerry609/PaperBot
Length of output: 43
🏁 Script executed:
cat web/src/lib/agent-events/parsers.test.tsRepository: jerry609/PaperBot
Length of output: 11930
🏁 Script executed:
rg "events/stream" web/src --type tsRepository: jerry609/PaperBot
Length of output: 296
Direct backend fetch from browser cannot authenticate and violates the established proxy pattern.
The hook calls fetch(${BACKEND_URL}/api/events/stream) directly from the browser, which bypasses the server-side auth layer. NextAuth tokens are available only on the server via the auth() function—the browser cannot access them. This fetch will fail authentication in any backend that requires a bearer token. Additionally, the http://localhost:8000 fallback on line 9 will route real users to their own localhost if the environment is missing.
Create a same-origin /api/events/stream route instead, using the proxyStream() utility with auth: true from backend-proxy.ts. This follows the established pattern where browser requests go through Next.js API routes, which inject credentials server-side before proxying to the backend.
While addressing this: note that the current retry logic (lines 47–50) only retries on network errors (caught exception). HTTP error responses like 401, 403, or 5xx return silently without reconnection, leaving the hook offline until reload.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/lib/agent-events/useAgentEvents.ts` at line 9, The hook is calling
fetch(`${BACKEND_URL}/api/events/stream`) from the browser (BACKEND_URL
constant) which bypasses server-side auth and uses a dangerous localhost
fallback; instead create a same-origin Next.js API route at /api/events/stream
that uses proxyStream({ auth: true }) from backend-proxy.ts, then change
useAgentEvents to fetch '/api/events/stream' (remove BACKEND_URL fallback). Also
update the reconnection logic in useAgentEvents so non-2xx HTTP responses
(401/403/5xx) are treated as errors that trigger the retry path (not just
network exceptions), ensuring the hook attempts reconnects rather than silently
staying offline.
Comment on lines
+21
to
+25
| const res = await fetch(`${BACKEND_URL}/api/events/stream`, { | ||
| signal: controller.signal, | ||
| headers: { Accept: "text/event-stream" }, | ||
| }) | ||
| if (!res.ok || !res.body) return |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n web/src/lib/agent-events/useAgentEvents.tsRepository: jerry609/PaperBot
Length of output: 2592
Missing retry logic for HTTP failures and clean stream closes.
fetch only throws on network or abort failures. A non-OK response at line 25 returns early, and a normal completion of the SSE loop at lines 28-46 also bypasses the error handler. This means a 401/503 or server-side stream closure leaves the dashboard permanently offline until reload. The retry logic at line 50 only handles exceptions, not these cases.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/lib/agent-events/useAgentEvents.ts` around lines 21 - 25, The SSE
connection abandons retries when fetch returns non-OK or when the stream cleanly
ends; update useAgentEvents.ts so the code treats non-OK responses and normal
stream completion as errors that trigger the existing retry/backoff logic: after
fetch(`${BACKEND_URL}/api/events/stream`, { signal: controller.signal, ... }) if
(!res.ok || !res.body) throw a descriptive Error (include status/text) instead
of returning, and inside the SSE reading loop detect EOF/closed stream and throw
or reject (unless controller.signal.aborted) so the outer catch/retry path at
the retry/backoff handler runs; keep the abort behavior intact so
controller.signal.abort still stops retries.
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
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.
What changed
web/src/app/apionto the shared helper pathfetchwrappersWhy
Validation
pytest -q tests/unit/test_next_backend_proxy_contracts.pycd web && npm exec vitest run src/app/api/_utils/backend-proxy.test.tsNotes
cd web && npm exec tsc --noEmitstill reports an existing unrelated failure insrc/lib/agent-events/parsers.test.tsdue to a missingtypefield in a test fixture. This PR does not touch that area.Summary by CodeRabbit
Release Notes
New Features
Documentation