Embed code review surface in frontend app#755
Open
backnotprop wants to merge 31 commits into
Open
Conversation
packages/plannotator-code-review — copy of packages/review-editor packages/plannotator-plan-review — copy of packages/editor Unmodified copies to start. These will be refactored to strip standalone providers (ThemeProvider, TooltipProvider, Toaster) and accept session-scoped API context from the frontend shell. The original packages remain untouched for the legacy single-file HTML flow.
React context that scopes fetch calls to a daemon session.
When inside a SessionProvider, fetch("/api/diff") rewrites to
fetch("/s/:sessionId/api/diff"). Without a provider, returns
the global fetch unchanged.
Add const fetch = useSessionFetch() to 10 hooks in packages/ui/hooks/. The shadowed fetch variable routes /api/ calls through the session context when a SessionProvider is present, and falls back to global fetch when not. configStore.ts uses apiFetch (import-based) since it's a class method.
Add const fetch = useSessionFetch() to all 11 files in
packages/plannotator-code-review/ that call fetch("/api/...").
The shadowed fetch variable routes calls through the session
context. No fetch call sites were modified — only the function
that provides the fetch was changed.
ReviewApp accepts __embedded prop to skip ThemeProvider, TooltipProvider, and Toaster (shell provides these). Uses h-full instead of h-screen when embedded. ReviewAppEmbedded is a named export that passes the prop. Default export unchanged. Shell Layout gains TooltipProvider for code review tooltips.
When session.mode === "review", the /s/:sessionId route wraps ReviewAppEmbedded in a SessionProvider and renders the full code review UI. Other modes keep the placeholder. - Added @plannotator/code-review as frontend dependency - Created App.d.ts type declaration for the code review package - Added plannotator-ui.d.ts for SessionProvider and ThemeProvider types - Added PNG module declaration for asset imports - Fixed settings.ts satisfies type for strict-mode compatibility - Vite alias resolves to package source for bundling - TypeScript uses .d.ts for type checking (avoids strict-checking loose package source)
- Add @custom-variant dark to code-review CSS for .light class toggle - Remove forced theme cookies from main.tsx (use defaultColorTheme prop) - Clean up document.title on review unmount (restore previous) - Clean up CSS custom properties on review unmount - Define __APP_VERSION__ from root package.json in vite config - Narrow Vite proxy to /s/:id/api/ only (page loads stay with Vite SPA) - Skip auto-registering temp directory projects in session factory - Add max-height scroll to project table for overflow - Add headerLeft prop to ReviewAppEmbedded for global sidebar trigger - Fix header padding when sidebar trigger is present
The code review's index.css had its own @import "tailwindcss" with separate @source and @theme directives, producing a second Tailwind build that competed with the frontend's styles.css. This caused buttons, dialogs, and other components to render with wrong styles. Fix: remove Tailwind, theme import, and @source from the code review's index.css (keep only dockview + custom CSS). Add @source directives to the frontend's styles.css to scan the code review package and shared UI components. One Tailwind build, one theme, no conflicts.
- Rename code review's @Keyframes fade-in to cr-fade-in to avoid collision with the frontend's fade-in (different animation) - Remove redundant panel scrollbar rules from styles.css (global scrollbar rules already cover all elements) - Add comment noting intentional scrollbar override of theme.css - Single Tailwind build, single theme import, zero duplications
- Strip machine-generated prefixes from session labels (plugin-review-, claude-code-, etc.) to show just the project/PR name - Add pr-7 padding to menu buttons so truncation ellipsis doesn't overlap with the status badge dot - Full label still visible on hover via tooltip
The useSessionFetch test replaced globalThis.fetch with a mock in beforeEach but never restored it. Other test files running in the same process (daemon runtime tests) got the mock instead of real fetch, causing JSON parse failures on "ok" responses. Added afterEach to restore the original fetch. All 1,463 tests pass.
Sessions now stay alive when the user navigates away. Instead of unmounting and remounting on each navigation, visited sessions are hidden via React's <Activity mode="hidden"> and restored instantly when the user returns. - AppStore tracks visitedSessions (keyed by session ID) and activeSessionId - Layout renders all visited sessions in <Activity> wrappers — only the active one is visible, the rest are hidden but preserved - Session route registers its bootstrap data with the store and renders nothing — Layout owns the rendering - Landing page deactivates the current session when navigated to - SessionSurface component extracted to handle mode-based rendering Effects clean up when hidden (WebSocket subs, timers stop) and restart when visible. DOM, React state, scroll position, annotations, dock layout all survive navigation.
Activity uses display:none which breaks Pierre diffs' virtualizer — it measures the container at zero height and renders no content. When made visible again, the virtualizer doesn't recalculate. Switch to visibility:hidden + position:absolute which preserves element dimensions. Pierre diffs keeps its measurements, Dockview keeps its layout. The tradeoff is effects don't pause for hidden sessions, but that's preferable to broken diffs.
Drafts are keyed by diff content hash on the server. Same diff = same draft. When a draft exists on mount, it's now restored automatically with a subtle toast notification instead of a blocking dialog. - useCodeAnnotationDraft takes an onRestore callback instead of returning draftBanner/restoreDraft/dismissDraft - ConfirmDialog for draft restore removed from App.tsx - Toast shows "Restored N annotations" on auto-restore
11 tasks
…cceeds - registerProject now finds existing entries by cwd (not name), so two repos with the same name get separate entries - removeProject takes cwd instead of name for unambiguous deletion - Server DELETE /daemon/projects now accepts JSON body with cwd - Session factory defers registerProject until after session creation succeeds, avoiding phantom entries from failed requests - Hub-client: add missing scheduleReconnect after protocol error frames
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
1 participant
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.
Summary
Layer 6 in the daemon stack. Embeds the production code review component inside the frontend app's session route, with session-scoped API routing via React context.
useSessionFetchhook +SessionProvider— React context that shadowsfetchwith a session-scoped version. When inside aSessionProvider,fetch("/api/diff")rewrites tofetch("/s/:sessionId/api/diff"). Fallback returns globalfetchwhen no provider is present. 9 unit tests.packages/ui/hooks/), 25 in code review package. Each getsconst fetch = useSessionFetch()at the top — existing fetch calls unchanged via variable shadowing.ReviewAppEmbeddedexport — strips ThemeProvider, TooltipProvider, Toaster (shell provides these). Keeps ReviewStateProvider + JobLogsProvider (internal). Usesh-fullinstead ofh-screen. AcceptsheaderLeftprop for global sidebar trigger.ReviewAppEmbeddedwrapped inSessionProviderwhensession.mode === "review". Other modes show placeholder.@import "tailwindcss"from code review CSS. Frontend'sstyles.cssscans all packages via@sourcedirectives.@keyframes fade-in, removed duplicate scrollbar rules, fixeddark:variant with@custom-variant dark~resolved in addProject, document.title and CSS properties cleaned up on unmount,__APP_VERSION__definedStack
feat/single-server-runtimefeat/plannotator-daemon-runtimefeat/runtime-frontend-shellfeat/websocket-event-hubfeat/frontend-initial-viewfeat/frontend-code-review-surfaceTest plan
bun run dev:frontend— daemon + frontend start/s/:id/s/:sessionId/api/.../s/:id— SPA loads (not debug shell)bun run --cwd apps/frontend checkpassesbun test— 1,460 pass (3 pre-existing flaky daemon runtime tests)